bears/haskell: Add GhcModBear for syntax checking #1267

vijeth-aradhya · 2017-01-06T14:30:09Z

Add a new bear in Haskell, namely GhcModBear for syntax checking.

Closes #297

gitmate-bot · 2017-01-06T14:30:25Z

Thanks for your contribution!

Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.

I have rebased properly. Please see this for a tutorial.
I have gone through the commit guidelines and I've followed them:
I have followed the guidelines for the commit shortlog.
I have followed the guidelines for the commit body.
I have included the issue URL.
I have used the Fixes keyword if the commit fixes a real bug and the Closes keyword if it adds a feature/enhancement. See more here.
I have run all the tests and they all pass. That includes that all CI (below) is green - if GitMate issues appear you will have to amend your commits, pushing new commits on top is not sufficient!

As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well!

Makman2 · 2017-01-06T15:56:35Z

alright there's a reason behind the travis error. You should try to pull our docker container and execute tests with and without your changes there, and see what's happening 👍

vijeth-aradhya · 2017-01-06T16:30:33Z

@Makman2 Is it because of my changes? I see this error in other pull requests as well :/
And, what's the reason probably?

vijeth-aradhya · 2017-01-06T16:41:21Z

@Makman2 And, I'm not sure how to use docker container for carrying out the tests. :/

Makman2 · 2017-01-06T20:32:45Z

Actually I can't believe your changes are causing this, though they reoccur, so let's eliminate the probability that your code is the cause^^

Docker is some kind of a leightweight VM:

Install Docker
docker pull coala/base
docker run -it coala/base bash

You get an interactive shell there. Then you clone coala and checkout your branch, install your new requirements, and execute tests -> py.test.

Adrianzatreanu · 2017-01-08T20:14:18Z

bears/haskell/GhcModBear.py

+                     r'(?P<message>.+)')
+class GhcModBear:
+    """
+    Syntax checking with ghc for Haskell files.


think "ghc" should be enclosed in double backticks

aptrishu · 2017-01-08T23:52:02Z

Ah some problem with LanguageToolBear and InferBear.. It's just travis' problem.

sils · 2017-01-12T08:23:48Z

bears/haskell/GhcModBear.py

+
+    @staticmethod
+    def create_arguments(filename, file, config_file):
+        return '-b', '. ', 'check', filename


what's the '. ' for?

can you put a comment in the code please? otherwise people will just remove it. Also do we have a test that would fail if this isn't given?

Ok I'll add a comment saying that :)
No, there is no test as such.

sils · 2017-01-12T08:28:04Z

code and tests look really proper, just that -b arg seems weird, waiting for response

vijeth-aradhya · 2017-01-12T12:04:20Z

The -b argument is to specify a separator for the messages as we have discussed. ( You can have a look at the asciinema for usage of . :) )
I had filed the issue here.

vijeth-aradhya · 2017-01-12T14:02:11Z

I have added the comment :)
No, there are no tests currently for . (for separation of messages).

vijeth-aradhya · 2017-01-13T06:21:18Z

I have modified a testcase to check for the . separator as discussed, but IMHO we should not check for exact error messages like that because different versions of ghc have slightly different error messages. (like in some versions it was `3' instead of ‘3’)
I'm pretty sure they won't remove it after putting a comment there ^

Thanks!

vijeth-aradhya · 2017-01-13T06:39:13Z

This is NOT a regex problem, but the output of ghc-mod differing :/
In the CI,

No instance for (Num String) arising from the literal `3'. Possible fix: add an instance declaration for (Num String). In the first argument of `putStrLn', namely `3'.

On my machine,

No instance for (Num String) arising from the literal ‘3’. In the first argument of ‘putStrLn’, namely ‘3’.

Since there is no universal output, it's better we don't test the . separator (IMO comment is sufficient instead of complicating stuff too much?!)

sils · 2017-01-13T07:13:05Z

can we test for . at least?

vijeth-aradhya · 2017-01-13T07:15:38Z

Initially, I had put only

No instance for (Num String) arising from the literal

because there was tick (') difference.
I can't think of how to check only for . though :3

vijeth-aradhya · 2017-01-15T06:11:19Z

I have added a vague test checking for . because the messages vary slightly according to ghc version. @sils
Please let me know if there any other changes :) I'd love to help and move on to other issues!

sils · 2017-01-15T10:06:43Z

please use the full URL in the shortlog, not just #..., otherwise it won't be linked in the git logs locally

sils · 2017-01-15T10:08:16Z

tests/haskell/GhcModBearTest.py

+                                      tempfile_kwargs={'suffix': '.hs'})
+        self.assertEqual(len(results), 1, str(results))
+        # to check for the seperator
+        self.assertTrue((len(str(results[0].message).split('.')) >= 4))


uhm this just checks that there are three .s right? Can't we just do a simple assertIn('. ' to see that the space is there properly?

sils · 2017-01-15T10:41:54Z

please use the full URL in the shortlog, not just #..., otherwise it won't be linked in the git logs locally

unack

Add ghc-mod feature to coala for syntax checking for haskell files (.hs) Closes coala#297

vijeth-aradhya · 2017-01-15T10:49:45Z

I did use the full URL :/ It turns into #.. after I push @sils

sils · 2017-01-15T11:06:38Z

you can see if it's the full URL if yo hover over the message and see the tooltip :)

sils · 2017-01-15T11:06:51Z

ack 20f7a1a

sils · 2017-01-15T12:23:51Z

@rultor merge

sils · 2017-01-15T12:23:58Z

🎉 finally

rultor · 2017-01-15T12:24:44Z

@rultor merge

@sils OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2017-01-15T12:26:07Z

@rultor merge

@sils Done! FYI, the full log is here (took me 1min)

vijeth-aradhya · 2017-01-15T12:51:23Z

You have no idea how relieved I am! 😄

gitmate-bot added process/pending review size/L review carefully! labels Jan 6, 2017

Adrianzatreanu reviewed Jan 8, 2017

View reviewed changes

aptrishu added process/wip and removed process/pending review labels Jan 8, 2017

vijeth-aradhya force-pushed the master branch from c9dc558 to 7cd564f Compare January 11, 2017 03:51

gitmate-bot added process/pending review and removed process/wip labels Jan 11, 2017

vijeth-aradhya force-pushed the master branch from 7cd564f to 429f04a Compare January 11, 2017 04:24

sils reviewed Jan 12, 2017

View reviewed changes

vijeth-aradhya force-pushed the master branch from 429f04a to 3909bed Compare January 12, 2017 12:04

vijeth-aradhya force-pushed the master branch from 3909bed to 46da83b Compare January 12, 2017 14:00

vijeth-aradhya force-pushed the master branch from 46da83b to 8a734fe Compare January 13, 2017 06:09

aptrishu added process/wip and removed process/pending review labels Jan 13, 2017

vijeth-aradhya force-pushed the master branch from 8a734fe to c1a3bc5 Compare January 15, 2017 05:26

gitmate-bot added process/pending review and removed process/wip labels Jan 15, 2017

vijeth-aradhya force-pushed the master branch 2 times, most recently from 4ac8fde to dbcbd74 Compare January 15, 2017 06:09

sils reviewed Jan 15, 2017

View reviewed changes

vijeth-aradhya force-pushed the master branch from dbcbd74 to 245e80c Compare January 15, 2017 10:22

coala-bot added process/wip and removed process/pending review labels Jan 15, 2017

bears/haskell: Add GhcModBear

20f7a1a

Add ghc-mod feature to coala for syntax checking for haskell files (.hs) Closes coala#297

vijeth-aradhya force-pushed the master branch from 245e80c to 20f7a1a Compare January 15, 2017 10:44

gitmate-bot added process/pending review and removed process/wip labels Jan 15, 2017

sils added process/approved and removed process/pending review labels Jan 15, 2017

rultor merged commit 20f7a1a into coala:master Jan 15, 2017

jayvdb mentioned this pull request Apr 28, 2017

bears/haskell: Add GhcModBear #1223

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bears/haskell: Add GhcModBear for syntax checking #1267

bears/haskell: Add GhcModBear for syntax checking #1267

vijeth-aradhya commented Jan 6, 2017 •

edited

Loading

gitmate-bot commented Jan 6, 2017

Makman2 commented Jan 6, 2017

vijeth-aradhya commented Jan 6, 2017

vijeth-aradhya commented Jan 6, 2017

Makman2 commented Jan 6, 2017

Adrianzatreanu Jan 8, 2017

aptrishu commented Jan 8, 2017

sils Jan 12, 2017

sils Jan 12, 2017

vijeth-aradhya Jan 12, 2017

sils commented Jan 12, 2017

vijeth-aradhya commented Jan 12, 2017 •

edited

Loading

vijeth-aradhya commented Jan 12, 2017 •

edited

Loading

vijeth-aradhya commented Jan 13, 2017 •

edited

Loading

vijeth-aradhya commented Jan 13, 2017 •

edited

Loading

sils commented Jan 13, 2017

vijeth-aradhya commented Jan 13, 2017

vijeth-aradhya commented Jan 15, 2017

sils commented Jan 15, 2017

sils Jan 15, 2017

sils commented Jan 15, 2017

vijeth-aradhya commented Jan 15, 2017

sils commented Jan 15, 2017

sils commented Jan 15, 2017

sils commented Jan 15, 2017

sils commented Jan 15, 2017

rultor commented Jan 15, 2017

rultor commented Jan 15, 2017

vijeth-aradhya commented Jan 15, 2017

bears/haskell: Add GhcModBear for syntax checking #1267

bears/haskell: Add GhcModBear for syntax checking #1267

Conversation

vijeth-aradhya commented Jan 6, 2017 • edited Loading

gitmate-bot commented Jan 6, 2017

Makman2 commented Jan 6, 2017

vijeth-aradhya commented Jan 6, 2017

vijeth-aradhya commented Jan 6, 2017

Makman2 commented Jan 6, 2017

Adrianzatreanu Jan 8, 2017

Choose a reason for hiding this comment

aptrishu commented Jan 8, 2017

sils Jan 12, 2017

Choose a reason for hiding this comment

sils Jan 12, 2017

Choose a reason for hiding this comment

vijeth-aradhya Jan 12, 2017

Choose a reason for hiding this comment

sils commented Jan 12, 2017

vijeth-aradhya commented Jan 12, 2017 • edited Loading

vijeth-aradhya commented Jan 12, 2017 • edited Loading

vijeth-aradhya commented Jan 13, 2017 • edited Loading

vijeth-aradhya commented Jan 13, 2017 • edited Loading

sils commented Jan 13, 2017

vijeth-aradhya commented Jan 13, 2017

vijeth-aradhya commented Jan 15, 2017

sils commented Jan 15, 2017

sils Jan 15, 2017

Choose a reason for hiding this comment

sils commented Jan 15, 2017

vijeth-aradhya commented Jan 15, 2017

sils commented Jan 15, 2017

sils commented Jan 15, 2017

sils commented Jan 15, 2017

sils commented Jan 15, 2017

rultor commented Jan 15, 2017

rultor commented Jan 15, 2017

vijeth-aradhya commented Jan 15, 2017

vijeth-aradhya commented Jan 6, 2017 •

edited

Loading

vijeth-aradhya commented Jan 12, 2017 •

edited

Loading

vijeth-aradhya commented Jan 12, 2017 •

edited

Loading

vijeth-aradhya commented Jan 13, 2017 •

edited

Loading

vijeth-aradhya commented Jan 13, 2017 •

edited

Loading