#124: change bracket-matcher:select-inside-brackets to ctrl-shift-m

[brunetton]

Two pull requests that goes together :

• markdown-preview: only activate ctrl-shift-m when in markdown editing context (to free that shortkey for all other grammars)
• bracket-matcher: make use of ctrl-shift-m for select-inside-brackets action

[thomasjo]

@brunetton Can you provide us with some justification behind this change? Is there some reason for changing the existing keybind? Thanks!

[brunetton]

@thomasjo: please have a look to #114, and (more complete story) #124. You'll understand the full story !

[thedaniel]

I think that we should be extremely conservative when assigning new keyboard shortcuts as they are a finite resource, and very vulnerable to the problem of many reasonable suggestions adding up to disorganization over the long term. This already has a keyboard shortcut and is not a 99th percentile action, so it is a perfect candidate for users to remap themselves in their own config. 👎 from me.

[brunetton]

@thedaniel I agree that each new keyboard shortcut should be assigned with caution.
So, why did the new keyboard shortcut proposed some days ago by @taylon in a patch without apparent thinking (cause it overlap long-time assigned ctrl-alt-m assigned by wild-used minimap) had been accepted without any questions in minutes ?

I'm a little scrambled: you want to keep a "new" (17 days ago; one Atom revision) shortkey that overlap an existing one in a wildly-used package ? (as you said, shortcuts are not an infinite resource) in favor to an existing one, more intuivie for everybody (at least @lloeki, @kevinsawicki, @cbenz and @dmnd that expressed themselves) that already exists ?

[thedaniel]

[So, why was the new keyboard shortcut proposed some days ago by @taylon accepted immediately]

Because there are thousands of open issues and pull requests, and not every one is looked at closely, unfortunately. I'll bring this up for discussion with the core team. If it was solely up to me I'd revert #126.

[dmnd]

@brunetton I think you messed up linking to issues that support your case - please check your links and fix them 😄

The justification for having ctrl-shift-m be the shortcut for select-inside-brackets is here: atom/markdown-preview#114. For convenience, I'll inline it:

I'm suggesting that markdown-preview use another keybind so that bracket-matcher can use ctrl-shift-m by default.

ctrl-m is used by bracket-matcher to navigate the cursor to a bracket. It makes sense for the shift key to convert this cursor movement action to a selection action, however ctrl-shift-m is already used by markdown-preview. Note also that ctrl-m and ctrl-shift-m are the keybinds used by Sublime too.

Another option would be to reduce the scope of markdown-preview's bind to only Markdown docs, but that would still prevent bracket matching working in Markdown docs and that might be annoying if the doc has code snippets in it. Also it's just generally inconsistent.

[brunetton]

@dmnd indeed :s I thought that referencing it on commit with "#124 would link the pull request to the issue", but no.
I referenced all the story in a comment on #124, and put that reference in the other pull requset that goes together with this one :)

Like I said in my long explaination post, I totaly agree with @dmnd's analysis

[cbenz]

What I'd like:

• ctrl-shift-m for selecting between matching brackets
• ctrl-alt-p for opening a preview (whatever file type is, svg, markdown, etc.)

The second point assumes that the shortcut is un-assigned from opening the specs test suite, unused by normal users.

[lloeki]

This looks fine to me.

[MoritzKn]

One thing to consider on this too maybe: Markdown doc can embed HTML where you maybe would still want to use the bracket-matcher functionality.