Fix haskell-ident-at-point uses and haskell-ident-pos-at-point behavior
#603
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The documentation mandates that `haskell-ident-at-point` return nil when there is no identifier at point; however, apparently, it instead returned `""` in that case at some point, and so some code made faulty assumptions based on that.
The documentation mandates that `haskell-ident-pos-at-point` return nil when there is no identifier at point; however, it instead returned a cons cell of the form `(n . n)`. This is now fixed, and any code that relied on this behavior has been changed. This also therefore fixes `haskell-spanable-pos-at-point` in the same way.
|
Awesome! (: Great job! |
|
Awesome! One more thing: there is a
|
| @@ -312,7 +312,7 @@ | |||
| (let ((ident (haskell-ident-at-point)) | |||
| (tags-file-name (haskell-session-tags-filename (haskell-session))) | |||
| (tags-revert-without-query t)) | |||
| (when (not (string= "" (haskell-string-trim ident))) | |||
| (when (and (stringp ident) (not (string= "" (haskell-string-trim ident)))) | |||
There was a problem hiding this comment.
Change:
(when (and (stringp ident) (not (string= "" (haskell-string-trim ident)))into
(when (and ident (not (string= "" (haskell-string-trim ident)))ident should be either nil or string at this point. I would prefer it to crash if it is something else rather than silently swallow wrong type.
As gracjan requested in haskell#603, I > * add[ed] a test case for haskell-ident-at-point really returning nil when it should > * add[ed] a test case for haskell-ident-pos-at-point returning non-nil > * add[ed] a test case for haskell-ident-pos-at-point returning nil > * add[ed] a test case for haskell-spanable-pos-at-point returning non-nil > * add[ed] a test case for haskell-spanable-pos-at-point returning nil I added some somewhat detailed `nil`-checking, a couple quick tests for `haskell-{ident,spanable}-pos-at-point` that just try the same unicode identifiers, and then some tests to distinguish `haskell-spanable-pos-at-point` by using an identifier in backticks. Any test for `haskell-ident-at-point` could also be a test for the other two, so that's a potential source of more tests; however, the definitions of the functions in terms of `haskell-ident-pos-at-point` are so simple that this didn't seem to add much (especially given the sheer amount of test duplication).
`haskell-mode-jump-to-tag` will now crash if given a non-`nil` non-string (inside `haskell-string-trim`), as gracjan requested in haskell#603.
gracjan
added a commit
that referenced
this pull request
Apr 25, 2015
Fix `haskell-ident-at-point` uses and `haskell-ident-pos-at-point` behavior
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The documentation mandates that
haskell-ident-at-pointandhaskell-ident-pos-at-pointreturnnilif there is no identifier at point. However:haskell-ident-at-pointinstead returned""in that case.haskell-ident-pos-at-pointinstead returns a cons cell of the form(n . n)in that case.This pull request fixes two things:
haskell-ident-at-pointalways returned a string; these assumptions have all been fixed. (Commit f8521d9.)haskell-ident-pos-at-pointnow returnsnilcorrectly, and any code relying on the old behavior was fixed. (Commit 6a3257c.)(Fixing
haskell-ident-pos-at-pointindirectly fixeshaskell-spanable-pos-at-pointin the same way.)This directly fixes issue #334. It also indirectly solves the problem in pull requests #601 and #602:
haskell-contextual-spacewas comparinghaskell-ident-at-pointto""instead ofnilat one point.