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

cider-jump with prefix to open buffer in other window? #1014

Closed
expez opened this issue Mar 13, 2015 · 28 comments
Closed

cider-jump with prefix to open buffer in other window? #1014

expez opened this issue Mar 13, 2015 · 28 comments

Comments

@expez
Copy link
Member

expez commented Mar 13, 2015

Wanting to read the source of one of the functions I'm calling is the most common use-case for me with cider-jump. To view the call-site and the function definition side-by-side I now have to do M-. C-x 3 C-x o C-x b <RET>. It would be nice if the jump would happen in the other window when cider-jump-to-var was given a prefix.

This might be cool for cider-jump-to-resource as well

@vspinu
Copy link
Contributor

vspinu commented Mar 13, 2015

This would be easy to implement. Both cider-jump-to-var and cider-jump-to-resource are using cider-jump-to which has an optional other-window argument that does exactly what you want.

The problem is with the usability. C-u prefix is already taken by cider-jump-to-var for manual insertion of the candidate. My personal opinion (explained in detail in #699) is that current behavior is non-standard and inefficient. Cider should always display a prompt with a default candidate like other emacs packages do. If #699 is fixed then current C-u is freed and could be used for something more useful like the other-window behavior that you propose.

@bbatsov
Copy link
Member

bbatsov commented Mar 13, 2015

I said it before - PRs welcome. I agree that a lot of things need polish, but sadly have little time to deal with this ATM.

On a related note - an alternative to the prefix would be alternative keybindings (like C-x 4 f) tied to alternative commands.

@vspinu vspinu mentioned this issue Mar 19, 2015
EricGebhart added a commit to EricGebhart/cider that referenced this issue Mar 20, 2015
@EricGebhart
Copy link
Contributor

@expez I have this working for both jump-to-var and jump-to-resource, but it's dependent upon #1031, so we'll have to wait to see how that goes. Not cool to pile up requests. In the mean time, it's in my repo if you want it.

@expez
Copy link
Member Author

expez commented Mar 20, 2015

@EricGebhart awesome! 👍 Just opened another issue, involving the code you're touching now, if you want more to do :)

Combine cider-jump-to-var and cider-jump-to-resource: #1032

EricGebhart added a commit to EricGebhart/cider that referenced this issue Mar 20, 2015
EricGebhart added a commit to EricGebhart/cider that referenced this issue Mar 21, 2015
EricGebhart added a commit to EricGebhart/cider that referenced this issue Mar 21, 2015
@malcolmsparks
Copy link

As far as I can tell, this means I now have to M-. (cider-jump) then hit return, into order to jump. This is hardly an improvement from just typing M-. - is there an alternative function I can bind to M-. ?

@vspinu
Copy link
Contributor

vspinu commented Mar 23, 2015

This is hardly an improvement from just typing M-. -

It's a big improvement if you get proper completions (see #699). The list of default completions should include not just the symbol at point but also the surrounding function call, class or method.

The actual completion list should ideally contain all symbols defined within a session.

@malcolmsparks
Copy link

I'd still like a quick way of navigating to the symbol under the point,
without having to get a mini-buffer prompt - is there a way that can be
achieved? nrepl-jump is the thing I used 10 times every minute, so I'm not
at all happy with this change.

On 23 March 2015 at 19:28, Vitalie Spinu [email protected] wrote:

This is hardly an improvement from just typing M-. -

It's a big improvement if you get proper completions (see #699
#699). The list of default
completions should include not just the symbol at point but also the
surrounding function call, class or method.

The actual completion list should ideally contain all symbols defined
within a session.


Reply to this email directly or view it on GitHub
#1014 (comment)
.

@EricGebhart
Copy link
Contributor

With #1036 we could use the extra binding to call a function that does not prompt. Jump-to-resource-or-var is currently mapped to both keybindings in that request. We could use the two bindings for one that prompts and one that doesn't.

It would also be nice to get the other completion values for the one that prompts. One step at a time.

@EricGebhart
Copy link
Contributor

So, if you had a choice. M-. or C-c M-. One that prompts and one that doesn't prompt. I would vote for M-. to prompt. But that's because I find the non-prompt version annoying. @danskarda might also be interested in this thread.

@malcolmsparks
Copy link

If I had a choice, definitely M-. for the no-prompt version, as it was
before. I'm know everyone has a unique development style, but for me,
nrepl-jump is 75% of the value of CIDER - without it I wouldn't even
install CIDER. I'm sure others who haven't yet updated will complain about
the regression of existing functionality to quickly jump into a symbol.

On 23 March 2015 at 21:26, Eric Gebhart [email protected] wrote:

So, if you had a choice. M-. or C-c M-. One that prompts and one that
doesn't prompt. I would vote for M-. to prompt. But that's because I find
the non-prompt version annoying. @danskarda https://github.com/danskarda
might also be interested in this thread.


Reply to this email directly or view it on GitHub
#1014 (comment)
.

@vspinu
Copy link
Contributor

vspinu commented Mar 24, 2015

Ok. I think we have to look into implementing a user option then, but that would make again an ugly code.

I'd still like a quick way of navigating to the symbol under the point,

You are ignoring the time it takes to navigate to the symbol you need and then back to the point you were before the request.

definitely M-. for the no-prompt version,

All emacs works with prompt, and for good reasons (again see #699). So I hope we are not reverting this.

@malcolmsparks
Copy link

All emacs works with prompt, and for good reasons (again see #699). So I hope we are not reverting this

You are citing your own comments here.

All emacs works with prompt??? No it doesn't. For example, Ctrl-F doesn't bring up a prompt. It would be seriously annoying if it did, asking how many characters you wanted to move forward.

I'm annoyed by this. It's seriously upset my development flow this evening, I just couldn't focus on writing code because this change forced me to look for a workaround, leading me to this issue.

If you are going to mess about with fixing things, do it on new key-bindings and convince people to migrate. Key-bindings are dear to me, they are wired in to my muscle memory, some of them as long as 20+ years I've been using Emacs. I don't recall a change ever feeling so unwelcome. And sooner or later my experience will be repeated by possibly hundreds, maybe thousands of other clojure emacs devs that rely on the current 'feel' of nrepl-jump. So I think this is worth taking into consideration. If I'm alone here, then so be it, I'll hack a workaround. But my prediction is that others will jump on this thread and complain about it, because it's a real annoyance. People use CIDER to get real work done, it's a major annoyance when CIDER devs do something that pulls the rug from under your feet.

@cichli
Copy link
Member

cichli commented Mar 24, 2015

Why not just have a user option for when to show the prompt? It seems like most people would just want all commands to consistently work in one of three ways:

  • always: the new behaviour, always show the prompt but default to symbol at point
  • dwim: only show the prompt if using the symbol at point failed
  • never: just try symbol at point and never prompt

I'll make a PR for this tomorrow if this sounds reasonable.

@malcolmsparks
Copy link

Sounds reasonable. I can accept the new functionality is useful in the case where your point isn't on a symbol. But if it is, then asking for a confirmation is terribly annoying. nrepl-jump is such an important navigation command for the Clojure dev when you have big projects, it's not something like finding a file where a prompt is expected. So your 'dwim' option seems the most sensible compromise, the next most sensible is providing user options - but the caveat is that it's one more thing for devs to diverge on and that makes working on other people's emacs setups that little bit more uncomfortable.

@EricGebhart
Copy link
Contributor

@chichli Are you proposing we have a setting for these three choices?

@bbatsov
Copy link
Member

bbatsov commented Mar 24, 2015

Sounds reasonable. I can accept the new functionality is useful in the case where your point isn't on a symbol. But if it is, then asking for a confirmation is terribly annoying.

It's also the norm in Emacs. :-) But yeah - I'm fine with having a config for this and I already suggested it. The previous default was a somewhat regrettable decision, but I guess it grew on people.

@cichli Green light from me for the suggested change.

@expez
Copy link
Member Author

expez commented Mar 24, 2015

It's also the norm in Emacs

Both you and @vspinu seem to think this is the case, but I don't think this is right. Emacs has two classes of jump commands:

  1. find-library, find-function, find-variable etc which always presents a prompt, but uses whatever is at point to provide a good default.
  2. Commands modeled after find-tag which act on the symbol at point and jump to that location.

All programming modes I use regularly have a variant of find-tag. And the traditional keybinding is M-. (as that is what find-tag uses). Here are a few examples:

robe-jump for ruby, elisp-slime-nav for elisp, slime-nav for common lisp, tern-find-definition for javascript and previously cider-jump-to-var for clojure.

I think we should add something covering 1) and keep the functionality in 2) intact (except for this request for a variant of find-tag-other-window).

Some examples of useful find-* we could use to solve 1) are find-ns and find-var. find-ns is a completing read of all available namespaces and find-symbol is a completing read of the union of all public vars.

@vspinu
Copy link
Contributor

vspinu commented Mar 24, 2015

Commands modeled after find-tag which act on the symbol at point and jump to that location.

Sorry to correct you on this but find-tag in emacs actually does prompt.

robe-jump for ruby, elisp-slime-nav for elisp, slime-nav for common lisp, tern-find-definition for javascript and previously cider-jump-to-var for clojure.

Just a guess, it started with slime and all others unfolded. Keeping it the default behavior in CIDER will enroot the problem further.

if you have a slime background then it's really difficult for you to see the inefficiency of that paradigm. You can clearly see the problem if you are brand new to it. It's especially bad for documentation commands.

I hate to sound repetitive, but let's think for a second, how do you navigate to the symbol? Probably C-n/C-p/M-f ... or isearch then M-.. Isn't it better to hit M-. first, type a couple of letters (ido assumed) then RET? Huge bonus - it keeps your working location intact!

@expez
Copy link
Member Author

expez commented Mar 24, 2015

[...] how do you navigate to the symbol?

I use ace-jump a lot, but I get your point. A very big part of my usage is actually looking up the symbol that I just wrote to answer the question "Does this really do what I think it does?".

When reading source I've actually picked up the habit of moving the cursor along with my eyes to make it easier to jump when the work is delegated and I want to follow that code path. Clearly some bad habits have been ingrained as a consequence of always acting on point.

@arrdem
Copy link
Contributor

arrdem commented Mar 25, 2015

Just my $0.02, but as a user of M-. primarily in the context of TAGS indexed C projects, I welcome being able to choose the target symbol at the prompt rather than only being able to jump to what I textually have at hand. An extra RET takes no time at all and I frankly don't understand the railing against it.

@j0ni
Copy link

j0ni commented Mar 25, 2015

I would vote strongly for @cichli's suggestion above, and feel @malcolmsparks' pain if not quite so viscerally.

@stathissideris
Copy link

@malcolmsparks thanks for arguing so strongly about this, this is important for me as well.

@oliyh
Copy link

oliyh commented Apr 2, 2015

+1 @malcolmsparks this is essential for frictionless development. It sounds like a polarising topic, so the option to bind the default key to either behaviour at the choice of the user sounds like the most sensible approach.

EricGebhart added a commit to EricGebhart/cider that referenced this issue Apr 4, 2015
EricGebhart added a commit to EricGebhart/cider that referenced this issue Apr 4, 2015
@enaeher
Copy link

enaeher commented Apr 5, 2015

I also really dislike the new default.

if you have a slime background then it's really difficult for you to see the inefficiency of that paradigm. You can clearly see the problem if you are brand new to it.

Do you have a Slime background? I think you are strongly underestimating how disruptive it is to have to continually double-check a prompt when you are rapidly using M-. and M-, to jump around. I usually use M-. several times in rapid succession as I follow the code flow several layers down, so it's not just one extra keystroke, it's double some arbitrary number of keystrokes.

I'd also like to request the re-enabling of the never option. I frequently use M-. only to figure out if a specific var is defined (for example, if I'm not sure whether I remember a function's name correctly). Prompting when there's no var means an extra C-g to get out of the useless prompt mode.

@bbatsov
Copy link
Member

bbatsov commented Apr 5, 2015

I'd also like to request the re-enabling of the never option. I frequently use M-. only to figure out if a specific var is defined (for example, if I'm not sure whether I remember a function's name correctly). Prompting when there's no var means an extra C-g to get out of the useless prompt mode.

This controls more that just M-., so it's not going to happen. I've said this many times already - people think of these commands as something supposed to operate just at point, but they are not. Not having to go to something to see its code or documentation is pretty handy IMO.

If you dislike it just change it - it's configurable for a reason. :-)

@bbatsov
Copy link
Member

bbatsov commented Apr 5, 2015

Do you have a Slime background? I think you are strongly underestimating how disruptive it is to have to continually double-check a prompt when you are rapidly using M-. and M-, to jump around. I usually use M-. several times in rapid succession as I follow the code flow several layers down, so it's not just one extra keystroke, it's double some arbitrary number of keystrokes.

Yes, I do. I've used it for over 5 years. That's the reason why much of the functionality in CIDER was initially modelled after SLIME. But CIDER is it's own application and follows its own path - the fact that something behaves in some way in SLIME doesn't make it the best/right way. There are plenty of people unhappy with some of SLIME's decisions (which eventually lead to the creation of the SLY project).

As for the usability problems of the default - it's just one more keystroke. I program a lot in Emacs Lisp and I'm quite used to this as its the standard Emacs behavior for commands like find-tag, describe-function, etc.

@enaeher
Copy link

enaeher commented Apr 5, 2015

people think of these commands as something supposed to operate just at point, but they are not

If that's the way people think of them, why not preserve the behavior that people expect? The principle of least surprise would dictate that if there is a widely-held expectation, the software should meet it. What is gained by intentionally breaking that principle here?

Not having to go to something to see its code or documentation is pretty handy IMO.

Agreed, but there is no need to break an existing keybinding to add this new feature. Why not leave the old behavior in place, with C-u M-. or (IMO preferably, since there is really little relationship between looking up the symbol at point and looking up an arbitrary symbol) some other unrelated keybinding for the new behavior?

@bbatsov
Copy link
Member

bbatsov commented Apr 5, 2015

If that's the way people think of them, why not preserve the behavior that people expect? The principle of least surprise would dictate that if there is a widely-held expectation, the software should meet it. What is gained by intentionally breaking that principle here?

You're assuming that most CIDER users are former SLIME users and that they actually enjoyed this particular SLIME behaviour. This may or may to be the case, but we have to concrete data to support such a claim. I accepted this change when it was suggested as the arguments in its favour were strong and sensible, not because I wanted to annoy anyone. People were given the ability to retain the old behaviour, so I don't see what more is there to discuss for such a trivial change.

Agreed, but there is no need to break an existing keybinding to add this new feature. Why not leave the old behavior in place, with C-u M-. or (IMO preferably, since there is really little relationship between looking up the symbol at point and looking up an arbitrary symbol) some other unrelated keybinding for the new behavior?

We discussed this as well, but we're already short on (good) free keybindings and we actually have to compact the existing ones (by packing them into submaps). There are only some many keys on a keyboard. :-)

P.S. We already added support for prefix args to the commands affected by the config - they invert the configured behaviour, which is a pretty good setup I think.

EricGebhart added a commit to EricGebhart/cider that referenced this issue Apr 6, 2015
EricGebhart added a commit to EricGebhart/cider that referenced this issue Apr 7, 2015
EricGebhart added a commit to EricGebhart/cider that referenced this issue Apr 19, 2015
bbatsov added a commit that referenced this issue Apr 20, 2015
[Fix #1014] Jump-to other window with prefix
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

No branches or pull requests