Initial attempts to add more haskell rules #180

volhovm · 2017-12-02T20:36:08Z

Alright, so I want to extend haskell support a little bit at least. This PR is currently WIP, because there are things I'm stuck with and I'd like somebody to help me (maybe). The main one is: does ag support multiline matching? I see a proof (here ggreer/the_silver_searcher#682) that it does, but for some reasons my tests show otherwise.

coveralls · 2017-12-02T20:38:47Z

Coverage remained the same at 97.314% when pulling 4a22321 on volhovm:master into c4b3e4b on jacktasia:master.

jacktasia · 2017-12-02T20:58:43Z

Thanks for the PR! To answer your question: my understanding is ag does have multiline support. Can you expand on your tests that show otherwise? I think #102 was the last time this came up so @netromdk may know more than me.

Thanks again!

netromdk · 2017-12-03T00:30:16Z

Yes, ag supports multiline matching. When I run your query on CLI then it works fine:

% ag "data.*{((.|\\s)*?::(.|\\s)*?,)*\\s*(test)\\b\\s*::(.|\\s)*}" test/test.hs
1:data Mem = Mem { mda :: A
2: , test :: Kek
3:  ,
4: aoeu :: E }

I put data Mem = Mem { mda :: A\n , test :: Kek\n ,\n aoeu :: E } in the file.

It fails through the :tests( .. ), though:

TEST: ’data Mem = Mem { mda :: A
 , test :: Kek
  ,
 aoeu :: E }’
INPUT: ’data Mem = Mem { mda :: A
 , test :: Kek
  ,
 aoeu :: E }’
CMD: ’ag --nocolor --nogroup data.\*\{\(\(.\|\\s\)\*\?\:\:\(.\|\\s\)\*\?\,\)\*\\s\*\(test\)\\b\\s\*\:\:\(.\|\\s\)\*\}’
RESP: ’’

volhovm · 2017-12-03T18:48:16Z

Alright, so as far as I've managed to figure out: ag doesn't support multiline when input is coming from stdin, so it has nothing to do with dumb-jump itself. IDK if it's possible to fix. Maybe we can change ag tests so they read things from the file directly (somehow, i'm not an elisp expert)?

netromdk · 2017-12-03T22:55:11Z

I actually did the same thing for git-grep: dumb-jump-run-git-grep-test. Maybe it could just be renamed to something like dumb-jump-run-test-from-file and be used inside dumb-jump-test-ag-rules. But then the temp file should be named something more general, of course. But I haven't tested it for ag in this multiline situation.

netromdk · 2017-12-04T19:12:45Z

Okay, if you do the following then it all works:

@@ -1212,9 +1212,9 @@ Optionally pass t for RUN-NOT-TESTS to see a list of all failed rules"
       (lambda (rule)
         (-mapcat
           (lambda (test)
-            (let* ((cmd (concat "ag --nocolor --nogroup "
+            (let* ((cmd (concat "ag --nocolor --nogroup --nonumber "
                                 (shell-quote-argument (dumb-jump-populate-regex (plist-get rule :regex) "test" 'ag))))
-                   (resp (dumb-jump-run-test test cmd)))
+                   (resp (dumb-jump-run-git-grep-test test cmd)))
               (when (or
                      (and (not run-not-tests) (not (s-contains? test resp)))
                      (and run-not-tests (> (length resp) 0)))

Except, of course, that dumb-jump-run-git-grep-test should be renamed as we talked about.

The deal was that while running this test it cannot show the line numbers, like:

1:data Mem = Mem { mda :: A
2: , test :: Kek
3:  ,
4: aoeu :: E }

That's why it needs --nonumber only for running the tests.

Edit: And it only starts showing line numbers when matching multiline input from stdin.

volhovm · 2017-12-04T21:39:35Z

Thank you, @netromdk. This is very helpful and I'll adopt it soon. I don't have a lot of time these days (was unsuccessfully testing my changes today) but I hope to finish this PR soon :)

volhovm · 2017-12-07T12:39:38Z

So I figured out I had a configuration mistake: my dumb-jump-force-searcher was set to auto which defaulted to git grep (though it wasn't obvious at all, i thought ag is preferred since it's more powerful).
I've solved the tests problem (yay, thanks everybody) and I will work on refining regexps themselves a little bit more.

coveralls · 2017-12-07T12:42:22Z

Coverage increased (+0.008%) to 97.323% when pulling 91b6a3e on volhovm:master into c4b3e4b on jacktasia:master.

netromdk · 2017-12-07T12:44:44Z

dumb-jump.el

@@ -1168,21 +1166,31 @@ If `nil` always show list of more than 1 match."
  "Use TEST as the standard input for the CMD."
  (with-temp-buffer
    (insert test)
-    (shell-command-on-region (point-min) (point-max) cmd nil t)
+    (shell-command-on-region (point-min) (point-max) cmd (current-buffer) t)


Is it necessary to use (current-buffer)? I can see in the doc on shell-command-on-region that:

If the value is nil, use the buffer ‘Shell Command Output’.

netromdk · 2017-12-07T12:46:15Z

Looks great! :)

volhovm · 2017-12-07T12:50:27Z

@netromdk is there any way i can match on something different from JJJ? Haskell types start with a capital letter, so Command rule should be different from command. I don't want some rules to fire up if substituted JJJ doesn't follow the internal predicate itself.

(i guess it's not possible, but asking anyway)

netromdk · 2017-12-07T18:01:43Z

Well, "JJJ" is just substituted the search term but it's up to the regexp to worry about case sensitivity. ag uses smart case by default, for instance:

  -S --smart-case         Match case insensitively unless PATTERN contains
                          uppercase characters (Enabled by default)

I think most rules kind of assume case-insensitivity. But at least for ag it should match correctly given "Command" or "command" in your case, but that's only when the regexp contains either "Command" or "command" because otherwise it would just substitute in the search term which doesn't directly affect the rule around it's case-matching mode.

Off the top of my head; If we wanted to support this then the rule itself would have to be annotated with a case-matching mode, "like starts with capital letter", which is matched against the value substituting in for "JJJ". This annotation could actually be a regexp in itself to weed out which search terms are even tried with the rule.

Food for thought.

What do you think about that, @jacktasia btw?

volhovm · 2017-12-07T18:02:19Z

I'm a little bit stuck. I have a set of rules. Pattern a is matched by regex/rule A. I add another rule B (which is not related to A) and a is not matched by anything anymore. b is also not matched by B (but when the exact same string appears in tests, it is).

Any ideas what it can be? Maybe there's some magic about the way rules are matched? It seems that adding extra rule (which perfectly works on tests i provide) ruins everything.

netromdk · 2017-12-07T18:06:59Z

That sounds weird, @volhovm. Do you mean it doesn't work with the inline rules but it works when you "jump" inside Emacs? And could you give an example of what's not working in code/examples.

coveralls · 2017-12-13T02:52:31Z

Changes Unknown when pulling ccb69df on volhovm:master into ** on jacktasia:master**.

coveralls · 2017-12-13T02:55:49Z

Changes Unknown when pulling ccb69df on volhovm:master into ** on jacktasia:master**.

volhovm · 2017-12-13T14:59:58Z

I think I'm actually done with this PR. Rules cover 99% of things i want, they work fast. I don't see any obvious problems.

coveralls · 2017-12-13T15:01:25Z

Changes Unknown when pulling 66cead4 on volhovm:master into ** on jacktasia:master**.

netromdk

Looks good. I'm looking forward to using your rules on my own Haskell programs!

netromdk · 2017-12-13T15:03:12Z

dumb-jump.el

@@ -699,7 +699,7 @@ or most optimal searcher."
           :tests ("newtype Test a = Something { b :: Kek }"
                   "data Test a b = Somecase a | Othercase b"
                   "type family Test (x :: *) (xs :: [*]) :: Nat where"
-                   "data family Test"
+                   "data family Test "


Is this space on purpose?

Yes. I believe that every identifier should be followed by a space or eof. \b doesn't work because in haskell we have a lot of things like Identifier and Identifier', so matching first will match second also.

jacktasia · 2017-12-13T20:55:53Z

@volhovm Thanks for all the work on this. I'll be merging it as soon as I get back to a computer. @netromdk Thanks so much for reviewing and helping on this and everything else you've done for dumb jump

netromdk · 2017-12-13T21:00:28Z

Oh, it's no problem. You're welcome :)

jacktasia · 2017-12-13T23:12:22Z

Thanks again!

Initial attempts to add more haskell rules

4a22321

Add support for multiline tests in ag

91b6a3e

netromdk reviewed Dec 7, 2017

View reviewed changes

volhovm added 3 commits December 13, 2017 05:36

Minimal working/decent configuration

9c83dde

Fix datatype/newtype constructor

636a4e1

Fix JJJ endings and type constructor haddock false positive

ccb69df

volhovm force-pushed the master branch from bba30b1 to ccb69df Compare December 13, 2017 02:49

Add rules for record fields

66cead4

netromdk approved these changes Dec 13, 2017

View reviewed changes

jacktasia merged commit a289063 into jacktasia:master Dec 13, 2017

dleslie pushed a commit to dleslie/dumb-jump that referenced this pull request Dec 14, 2017

add more haskell rules (jacktasia#180)

053dad3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial attempts to add more haskell rules #180

Initial attempts to add more haskell rules #180

volhovm commented Dec 2, 2017

coveralls commented Dec 2, 2017 •

edited

Loading

jacktasia commented Dec 2, 2017

netromdk commented Dec 3, 2017

volhovm commented Dec 3, 2017

netromdk commented Dec 3, 2017

netromdk commented Dec 4, 2017 •

edited

Loading

volhovm commented Dec 4, 2017

volhovm commented Dec 7, 2017

coveralls commented Dec 7, 2017 •

edited

Loading

netromdk Dec 7, 2017

volhovm Dec 7, 2017

netromdk commented Dec 7, 2017

volhovm commented Dec 7, 2017

netromdk commented Dec 7, 2017 •

edited

Loading

volhovm commented Dec 7, 2017 •

edited

Loading

netromdk commented Dec 7, 2017 •

edited

Loading

coveralls commented Dec 13, 2017 •

edited

Loading

coveralls commented Dec 13, 2017 •

edited

Loading

volhovm commented Dec 13, 2017

coveralls commented Dec 13, 2017 •

edited

Loading

netromdk left a comment •

edited

Loading

netromdk Dec 13, 2017

volhovm Dec 13, 2017

jacktasia commented Dec 13, 2017

netromdk commented Dec 13, 2017

jacktasia commented Dec 13, 2017

Initial attempts to add more haskell rules #180

Initial attempts to add more haskell rules #180

Conversation

volhovm commented Dec 2, 2017

coveralls commented Dec 2, 2017 • edited Loading

jacktasia commented Dec 2, 2017

netromdk commented Dec 3, 2017

volhovm commented Dec 3, 2017

netromdk commented Dec 3, 2017

netromdk commented Dec 4, 2017 • edited Loading

volhovm commented Dec 4, 2017

volhovm commented Dec 7, 2017

coveralls commented Dec 7, 2017 • edited Loading

netromdk Dec 7, 2017

Choose a reason for hiding this comment

volhovm Dec 7, 2017

Choose a reason for hiding this comment

netromdk commented Dec 7, 2017

volhovm commented Dec 7, 2017

netromdk commented Dec 7, 2017 • edited Loading

volhovm commented Dec 7, 2017 • edited Loading

netromdk commented Dec 7, 2017 • edited Loading

coveralls commented Dec 13, 2017 • edited Loading

coveralls commented Dec 13, 2017 • edited Loading

volhovm commented Dec 13, 2017

coveralls commented Dec 13, 2017 • edited Loading

netromdk left a comment • edited Loading

Choose a reason for hiding this comment

netromdk Dec 13, 2017

Choose a reason for hiding this comment

volhovm Dec 13, 2017

Choose a reason for hiding this comment

jacktasia commented Dec 13, 2017

netromdk commented Dec 13, 2017

jacktasia commented Dec 13, 2017

coveralls commented Dec 2, 2017 •

edited

Loading

netromdk commented Dec 4, 2017 •

edited

Loading

coveralls commented Dec 7, 2017 •

edited

Loading

netromdk commented Dec 7, 2017 •

edited

Loading

volhovm commented Dec 7, 2017 •

edited

Loading

netromdk commented Dec 7, 2017 •

edited

Loading

coveralls commented Dec 13, 2017 •

edited

Loading

coveralls commented Dec 13, 2017 •

edited

Loading

coveralls commented Dec 13, 2017 •

edited

Loading

netromdk left a comment •

edited

Loading