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

Add cider-prompt-for-symbol option #1044

Merged
merged 1 commit into from
Mar 26, 2015

Conversation

cichli
Copy link
Member

@cichli cichli commented Mar 24, 2015

As proposed in the comments of #1014. Defaults to the old behaviour currently - what should it be?

@expez
Copy link
Member

expez commented Mar 24, 2015

dwim looks good to me, it's extending existing functions with new behavior without breaking anything that previously worked.

@cichli
Copy link
Member Author

cichli commented Mar 24, 2015

Oh, it is slightly different from the old behaviour in that it now calls the command with symbol at point first (rather than checking if it's nil or an empty string), and if that throws it will not default to symbol at point in the prompt.

No prefix arg option currently but it would be easy to have a prefix arg always show prompt (or perhaps do the opposite of what it would normally do, based on the current value of cider-prompt-for-symbol).

@vspinu
Copy link
Contributor

vspinu commented Mar 24, 2015

DWIM could be more explicit, like skip-prompt. DWIM is only for users who already got used to this behavior.

Could you please make it default to the emacs standard way. The defaults should take into account not only the previous users but new users still to come. It will discourage other IDEs using CIDER as an example to motivate this historical derail.

Another problem with dwim is that it's a horrible default for the doc lookup.

@bbatsov
Copy link
Member

bbatsov commented Mar 25, 2015

I've been thinking about this yesterday some more and thought that having different commands that operate at point or prompt always might be a better option.

The problem with the options approach is that setting the new style would make it hard to operate at the point (an extra keypress) and the old one makes everything else hard (people commonly go to some symbol to lookup its documentation/definition). A prefix argument might solve this, but most people simply don't notice them.

Could you please make it default to the emacs standard way. The defaults should take into account not only the previous users but new users still to come. It will discourage other IDEs using CIDER as an example to motivate this historical derail.

Whether all Emacs defaults are good is a pretty subjective matter. :-)

@cichli
Copy link
Member Author

cichli commented Mar 25, 2015

The problem with the options approach is that setting the new style would make it hard to operate at the point (an extra keypress) and the old one makes everything else hard

Isn't learning a new set of keybindings (and two keybindings instead of one for all new commands that require a symbol) more frustrating than learning one rule: C-u makes this type of command do the opposite of your configured default?

A prefix argument might solve this, but most people simply don't notice them.

I think this is just a problem of documentation. If it's well documented and consistent across all "jump-to" type operations that using a prefix arg will do the opposite of what you have configured, then it's quick to learn and not much mental overhead.

If we introduce new commands, we now have two commands for every one of these operations, and now have to come up with sensible new keybindings for the new ones. Perhaps we can use . as the generic "try symbol at point" suffix, so we have C-c C-d d / C-c C-d . for prompt/dwim. But this doesn't work for all commands (e.g. grimoire is already behind C-c C-d so would need a new prefix, which means changing even more keybindings, which will only further alienate existing users).

So we'd either be forcing people to learn a bunch of new keybindings or (far more likely) remap a bunch of keys in their configuration. This worsens the problem that @malcolmsparks mentioned in #1014 of pairing using someone else's setup being difficult. If I'm using someone else's setup, I'd much rather just have to note "I need to C-u before using jump commands" than "I have to use all these other keybindings that I never normally use". It's also possible to just temporarily change cider-prompt-for-symbol - not really the case if keys are remapped.

The option itself could probably just be simplified to non-nil for always prompt and nil for the dwim behaviour. I'm not sure if never prompting (even if symbol at point is not resolved) is that useful.

@vspinu
Copy link
Contributor

vspinu commented Mar 25, 2015

I don't see the point of two sets of almost identical commands. Not many people will use both paradigms simultaneously.

@arrdem
Copy link
Contributor

arrdem commented Mar 25, 2015

Just my $0.02, I think that having two sets of prefix commands differing only in whether they prompt or not is kinda silly. Agreed that it's the worst degenerate case of users wanting to prompt sometimes but I consider prompting to be a sane default and no big deal. I'd stick with the global setting.

@EricGebhart
Copy link
Contributor

Is anyone actually complaining about anything other than jump-to-var? I've looked through all the threads and don't see any reference to anything else. Of course I think prompting makes perfect sense in all cases, but especially in the case of doc, the frequency of wanting to get doc on something you don't have at point is much higher than jump-to-var which is mostly likely somewhere close by. My annoyance there is I don't want to move my cursor just so jump to var will actually work.

This also effects cider-undef in which case, I think prompting is a good thing, just to make sure I'm undefining the thing I think I am.

I have been looking through etags.el to see what it does because it seems completely sane to me.
It does prompt.

Up until this week I have never used jump-to-var. I couldn't see the point, ha. I use find-tag. So far I can't tell there is a difference in performance other than find-tag works without cider running.

Etags prompts when I want it to and not when I don't. Their code for interactive is quite nice and works for all scenarios. And they use the standard interactive interface unlike cider-read-symbol-name.

Etags does have a lot more functionality which this code also deals with. A prefix to find next and a prefix of - to go back to the previous match among others. Neither of those prompt as you would expect.

@EricGebhart
Copy link
Contributor

Is jump-to-var the only real issue here? Because if that is the case, #1032 could address this whole thing. @expez wants an easy way to open jump-to-var or resource in another window. There is one function in #1032 that does jump-to-resource-or-var. Currently with a prefix it jumps to other window.
jump-to-var and jump-to-resource have been turned into non-interactive functions.
That gives us one extra keybinding. M-. could be jump-to and C-c M-. could be jump to other window. Then a prefix to cause either one not to prompt. That is at least consistent with etags if nothing else. If that is still too much of an annoyance one variable that says not to prompt for jump-to var should be sufficient.

@expez
Copy link
Member

expez commented Mar 25, 2015

@EricGebhart My patience is growing thin at this point. The exact behavior I want is never more than a few minutes of hacking away in emacs so I feel this analysis paralysis is out of control.

That gives us one extra keybinding. M-. could be jump-to and C-c M-. could be jump to other window. Then a prefix to cause either one not to prompt.

I'd prefer the prefix control the other window stuff and a variable customizing the behavior of the prompt, because hitting RET to just skip past the prompt and hitting C-u to say "don't prompt" is the same amount of keys pressed. I like the 3 proposed options for customizing the prompting and I'll stick it to dwim myself, or whatever it ends up being called.

@EricGebhart
Copy link
Contributor

@expez like you said, it's all pretty easy to do in emacs. I think the irritability of this is blinding some to logical thought. Overly complex configuration schemes are seldom a good idea. The next thing you know we'll be having configurations to turn prompts on for this and off for that. The doc-prompt change is only implicated because in addition to fixing the doc commands it also changed the behavior of jump-to-var.

I contend that if jump-to-var had not changed this conversation would not be happening.

I should also point out @expez that #1032 currently is coded exactly the way you want. It prompts, but that is because I coded it to be consistent with the previous changes to cider-read-symbol-name. Which is the problem being discussed here. jump-to-resource-or-var has it's own interactive and so could be coded to do whatever we want. Further, it could be done separately from all this prefix / key-binding, multiple configuration variables mess.

Either way this thing goes, I'll have to recode #1032 to match. I'm just pointing out that it could be a lot simpler than what this looks to be.

@cichli
Copy link
Member Author

cichli commented Mar 25, 2015

I contend that if jump-to-var had not changed this conversation would not be happening.

This is probably true, and if all people want is for M-. to do what it used to then I'm more than happy for a simpler solution to be put into place than what I have proposed here. FWIW I'm not particularly attached to either way. I just intended to provide a solution that gave both consistent behaviour across all commands and the ability to override temporarily with C-u (I was expecting that C-x 4 would be used for the -other-window prefix).

@EricGebhart
Copy link
Contributor

@cichli, I'm with you on all counts. I'm also not particularly attached. Once it's settled I can code #1032 to match.

@bbatsov
Copy link
Member

bbatsov commented Mar 26, 2015

@cichli OK, let's just add prefices to the commands which flip their meaning. While not ideal, this should make pretty much everyone happy (or at least I hope so).

@cichli
Copy link
Member Author

cichli commented Mar 26, 2015

Done.

  • The never option is gone. Now just non-nil/nil for new/old behaviour.
  • Defaults to always prompting.
  • All relevant commands now accept a prefix arg to flip meaning.

bbatsov added a commit that referenced this pull request Mar 26, 2015
Add `cider-prompt-for-symbol` option
@bbatsov bbatsov merged commit 9492a05 into clojure-emacs:master Mar 26, 2015
@bbatsov
Copy link
Member

bbatsov commented Mar 26, 2015

👍

@EricGebhart
Copy link
Contributor

Sweet! Thank You!

@cichli cichli deleted the prompt-option branch March 30, 2015 17:21
@lgrapenthin
Copy link

I find that the prompt should be turned off by default, in case anybody still cares. After upgrading, it was irritating and I was lucky to find this issue. I have no reasons other than that the prompt is a different feature than a contextual jump command, even if it is a contextual prompt. I did only scan the discussion about emacs defaults, but I know that when I hit enter on a file in dired, it doesn't prompt me either.

@lgrapenthin
Copy link

Also, in customize, the nil option is for some reason called "dwim".

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.

7 participants