jyp / dante

389 stars 52 forks source link

Tell flycheck checker where project root is for error resolution purposes #170

Closed sergv closed 1 year ago

sergv commented 1 year ago

Otherwise flycheck uses current buffer's directory to resolve paths in errors reported by GHC which is not always correct thing to do.

jyp commented 1 year ago

I think that dante-ghci-path should be used as a root instead of dante-project-root (they can be different for multi-package projects). Does this make sense?

sergv commented 1 year ago

I suppose dante-ghci-path is good too.

However, upon closer examination the dante-ghci-path seems to be unset when the checker reads it. With flycheck the checker will be initialized before dante has started ghci.

jyp commented 1 year ago

Ok, I'm merging this PR. It appears to be an improvement even though it's probably not the final word.

BTW, I've been considering deprecating flycheck in favour of flymake. Is there any reason for you to use flycheck specifically?

sergv commented 1 year ago

It appears to be an improvement

Unless something changes it doesn't do anything since the variable is nil at the time it's accessed and gets ignored.

Is there any reason for you to use flycheck specifically?

I don't know enough about flymake to make an informed decision, but generally I'm using flycheck for unified handling of checkers across different languages - it provides the same interface to many checkers. I have some custom functions to jump to next/previous flycheck error, I get some benefit from flycheck's ability to use multiple checkers together - i.e. after running dante it will also run hlint. Checkers can actually be configured with working directory and flycheck is able to resolve a sutiation when error occurs in different file from the one I'm editing right now and take me there.

I'll consider switching to flymake if it can support all the features I care about and can do something that flycheck cannot, otherwise it's swapping alike for alike but requires spending effort. I won't stop you from removing flycheck - I can define flycheck checker for dante if need be.

jyp commented 1 year ago

Thinking about your problem more, it should be possible to make the path absolute by using the ghci-path at the time that the error message is produced. Probably in dante-fly-message.

Unless something changes it doesn't do anything since the variable is nil at the time it's accessed and gets ignored.

Ok, I've done it like you suggested first and noted the possible problem in comment, as well as noted the possible way to fix the problem long run. It would be helpful if you could make a bug report with a minimal way to reproduce the problem.

About flymake vs flycheck:

unified handling of checkers across different languages

Flymake can do this.

flycheck's ability to use multiple checkers together [chained]

Flymake can run checkers in parallel. IMO it's at least as good.

flycheck is able to resolve a situation when error occurs in different file

Flymake has the API for this (in its latest version). Again, if you have a minimal example where this happens for Dante, please open a bug report.