-
Notifications
You must be signed in to change notification settings - Fork 347
Invoke hasktags more directly #1204
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
Conversation
(ignore-errors (when (and (boundp 'haskell-session) haskell-session) | ||
(haskell-process-generate-tags)))) | ||
(ignore-errors | ||
(if (and (boundp 'haskell-session) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need for both functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, yes, at least for now, look at this
https://github.com/haskell/haskell-mode/blob/master/haskell.el#L351
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we can rewrite jump-to-tag also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jump to tag generates tags? That is surprising.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this was my not idea :D
But I think it's not so bad at the end, otherwise user have to generate tags manually. Do you think we should remove tags generation from jump-to-tag? (I suppose there are more surprising things awaiting :D)
Ok, I'll do that now |
@gracjan done |
Looks great! I wonder if we could have a testing framework for all these external programs that we invoke here and there. We could have some integration tests in There are a couple of ways around that issue:
I would hate if |
I've filled in #1206 to track the integration testing stuff. |
@gracjan what about this one? |
I've done code review and code is good. I did not have time to actually run the code though. Is there anybody else beside you @geraldus that has checked if it really works? |
(when buffer-file-name | ||
(file-name-directory buffer-file-name)))) | ||
|
||
(defun haskell-cabal--compose-hasktags-command (&optional cabal-dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make cabal-dir
non-optional, always require this to be a proper directory.
@geraldus, after you fix the one thing I found then we can merge this. Rest looks great. |
If session is present then use asynchrounous tags generation function from interactive package, otherwise use blocking function from static package.
Traversing directories is potentially very costful action. This function helps to re-use found dir in some cases.
Move `haskell-mode-message-line` and `haskell-mode-one-line` to haskell-mode.el. This function are not specific to interactive package, also both have name which nicely fit to haskell-mode.el rather than haskell-interactive-mode.el. This movement is required to for static tags generation function also.
Support optional parameter to jump to indentifier definition after tags were generated.
🍵 All is green now |
Nice! |
#964