Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

#114: only activate ctrl-shift-m keybinding when editing MarkDown source #249

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brunetton
Copy link

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

@mnquintana
Copy link
Contributor

👍 I think this is a good idea, but it does change the usage of the command a bit (now you have to have focused an editor, when before it worked anywhere in the workspace), so I'd like other people to weigh in on this first. /cc @atom/feedback

@brunetton
Copy link
Author

Yes indeed.
It also could make be programmatically in js code in a second time if people care about having to focus to editing pane before doing the shortcut: if active editor file is markdown, then enable the shortcut, else ignore it.
But I don't think anybody cares. Asking for markdown-preview in tree-view pane is not natural, and don't see any situation where it is natural to execpt markdown-preview shows up while not focused in editing pane. Do you ?

I'm quite afraid that the nedds-review state will stuck this pull request into an infinite pending state, like I can see for this one :s

@mnquintana mnquintana self-assigned this May 11, 2015
@mnquintana
Copy link
Contributor

I'm quite afraid that the nedds-review state will stuck this pull request into an infinite pending state, like I can see for this one :s

Don't worry, that's not how the needs-review label works - far more PRs have been reviewed with that label than not. Plus I'm keeping an eye on this 😉

But I don't think anybody cares. Asking for markdown-preview in tree-view pane is not natural, and don't see any situation where it is natural to execpt markdown-preview shows up while not focused in editing pane. Do you ?

I agree, but I want to get a bit more feedback before I merge. I'm definitely 👍 for this change though.

@brunetton
Copy link
Author

Thanks!
Le 11 mai 2015 13:57, "Machisté N. Quintana" [email protected] a
écrit :

I'm quite afraid that the nedds-review state will stuck this pull request
into an infinite pending state, like I can see for this one :s

Don't worry, that's not how the needs-review label works - far more PRs
have been reviewed with that label than not. Plus I'm keeping an eye on
this [image: 😉]

But I don't think anybody cares. Asking for markdown-preview in tree-view
pane is not natural, and don't see any situation where it is natural to
execpt markdown-preview shows up while not focused in editing pane. Do you ?

I agree, but I want to get a bit more feedback before I merge. I'm
definitely [image: 👍] for this change though.


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

@thomasjo
Copy link
Contributor

I'm 👍 on this PR. I've posted a question on the linked PR.

@brunetton
Copy link
Author

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

@lloeki
Copy link

lloeki commented May 11, 2015

@brunetton said in #114:

just change key binding to something else (I've no idea for now)

This might be the moment to propose that *-preview (where, * is markdown, svg, image or whatever) could have a common "open preview" keybinding tentatively uniform across the board.

@lee-dohm
Copy link
Contributor

I like @lloeki's idea of a "preview" key.

@thedaniel
Copy link
Contributor

Can we be sure that a user will always have a 'gfm' grammar when they want a preview?

@kevinsawicki
Copy link
Contributor

This was discussed on a previous pull request that is very similar to this one: #175 (comment)

@brunetton
Copy link
Author

Indeed @kevinsawicki, I didn't get it. Thanks for pointing this out. I didn't checked wether "markdown-preview" package would be useable for html and cofeescript :)

So, the css selector data-grammar="source gfm" isn't valid.
So, for now, we can't imagine make use of ctrl-shift-m.

Some possible next steps then :

  • adapt css selector to other grammars, assuming that, like @lloeki said : "There are not much brackets in markdown"
  • change "markdown" preview key to a generic preview key; like proposed by @lloeki, that would free ctrl-shift-m (or not), then if free, apply the linked pull request (simply change select between brackets shortkey from ctrl-alt-m to ctrl-shift-m)
  • change old and well established bracket-matcher ctrl-m shortcut (I realy don't like it)
  • totaly forget the idea of being logic with this shortcut (ctrl-m to jump => ctrl-shift-m to select between matchings), and aligned to sublimed text (I don't like it neither)

@mnquintana mnquintana removed their assignment Jun 12, 2015
@brunetton
Copy link
Author

Any news on this ?

@sukrosono
Copy link

I am really happy to see @brunetton 's works merged.

Forgive me if I mention the wrong person.
ping to @nathansobo @rafeca

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants