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

Don't move point when clearing or highlighting test results #717

Merged
merged 1 commit into from
Aug 21, 2014
Merged

Don't move point when clearing or highlighting test results #717

merged 1 commit into from
Aug 21, 2014

Conversation

r0man
Copy link
Contributor

@r0man r0man commented Aug 8, 2014

When running tests with cider-test failing tests are highlighted in the source
files, or if tests are passing again previous highlights are cleared. The code
that does this is using the cider-find-var function to find the buffer for a
vars under test and then does it's job.

cider-find-var used to move point to the line at which the var was found. This
is a problematic side effect, because code using cider-find-var to find the
buffer can't use save-excursion because it doesn't know which buffer to safe
beforehand.

This PR removes the goto-line in cider-find-var and returns just the buffer
without any side effect. The cider-company-location uses similar code as
cider-find-var but with save-excursion wrapped around it.

@bbatsov
Copy link
Member

bbatsov commented Aug 8, 2014

Seems to me that your change will affect the behavior of C-..

@r0man
Copy link
Contributor Author

r0man commented Aug 8, 2014

cider-jump-to-var seems to still work for me. What do you mean exactly?

@@ -2,6 +2,11 @@

## master (unreleased)

### Bugs fixed
Copy link
Member

Choose a reason for hiding this comment

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

There's already a Bugs fixed section beneath Changes.

@r0man
Copy link
Contributor Author

r0man commented Aug 8, 2014

Fixed the duplicate "Bugs fixed" section and rebased.

@bbatsov
Copy link
Member

bbatsov commented Aug 8, 2014

Ah, the name cider-find-var confused me; someone should change this to cider-find-var-buffer or something similar... Guess this was done solely for company's benefit then.

👍 from me, but let's wait to hear from @jeffvalk as well because he recently refactored this code.

@bbatsov
Copy link
Member

bbatsov commented Aug 12, 2014

@jeffvalk Ping :-)

@jeffvalk
Copy link
Contributor

Pardon my tardiness on this! Yes, this change makes sense to me. @r0man Would you update this PR to apply against HEAD?

I also agree with @bbatsov that the name cider-find-var should be changed to reflect the revised behavior. Since it would now essentially mimic cider-find-file, perhaps cider-find-var-file makes sense.

When running tests with cider-test failing tests are highlighted in the source
files, or if tests are passing again previous highlights are cleared. The code
that does this is using the `cider-find-var` function to find the buffer for a
vars under test and then does it's job.

`cider-find-var` used to move point to the line at which the var was found. This
is a problematic side effect, because code using `cider-find-var` to find the
buffer can't use save-excursion because it doesn't know which buffer to safe
beforehand.

This PR removes the `goto-line` in `cider-find-var` and returns just the buffer
without any side effect. The `cider-company-location` uses similar code as
`cider-find-var` but with `save-excursion` wrapped around it.
@r0man
Copy link
Contributor Author

r0man commented Aug 21, 2014

@jeffvalk & @bbatsov I rebased this on master

bbatsov added a commit that referenced this pull request Aug 21, 2014
Don't move point when clearing or highlighting test results
@bbatsov bbatsov merged commit 7b7601b into clojure-emacs:master Aug 21, 2014
@bbatsov
Copy link
Member

bbatsov commented Aug 21, 2014

👍

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.

3 participants