Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ghc-mod linter check current buffer #846

Merged
merged 1 commit into from
Aug 13, 2017

Conversation

voanhduy1512
Copy link
Contributor

Right now ghc-mod linter check temp file instead of current buffer,
which cause the problem that it can't detect cabal file and raise
missing package error.

IP/s: try to look at the test but can't find ghc-mod specific test.

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two options here.

  1. Also set 'lint_file': 1, so only files on disk will be checked on open, save, etc.
  2. Figure out if there's some way to get this working with a temporary file, maybe by using cd ... &&, or passing an argument for telling GHC where the Cabal file is, or by passing some argument to say where the actual file is, etc.

The second option is preferable, if possible.

@voanhduy1512
Copy link
Contributor Author

Looks like ghc-mod have a map-file option that Redirect one file to anotheror from stdin source. I assume with this i can implement 2nd option as your suggestion. I will update my PR soon.

@w0rp
Copy link
Member

w0rp commented Aug 13, 2017

Cool, that might do the trick. 👍 flake8, mypy, and eslint have similar options.

Right now ghc-mod linter check temp file instead of current buffer,
which cause the problem that it can't detect cabal file and raise
missing package error.

To fix that we need to run ghc-mod check with actual path of the current
file and with ghc-mod option `--map-file` to redirect temp file source
code to actual one
@voanhduy1512
Copy link
Contributor Author

Ready for review 🎉

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this looks like a good solution. Thanks!

@w0rp w0rp merged commit a5d7bb4 into dense-analysis:master Aug 13, 2017
@w0rp
Copy link
Member

w0rp commented Aug 13, 2017

Cheers! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants