-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Renaming commands #598
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
Renaming commands #598
Conversation
|
👍 for having a consistent naming-style guide on these.
This sentence is a bit ambiguous to me. Is the intent here equivalent to "Use above/below to indicate spacial relationships. Use previous/next to indicate sequential relationships."? Is sequential indended as a synonym for "temporally-sequential" here? |
|
Yeah, when to use above/below or previous/next can be a bit sutble and that difficulty is reflected in the sentence structure. |
Why that ? Especially since we have multiple selection and anchor ? I fel a lot confused when searching for an action if there is not the "selected", or "all" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ipython: ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opps, good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually jupyter-notebook
|
@Carreau as to why omit the "selected". Initially we started with that, but it felt one-step-too-verbose, when a significant fraction of the time, the user will be applying actions to the active object. Another thing that we talked about today. For actions that make sense on multiple (marked) cells, the simple versions of commands should apply to:
The alternative is to have 3 versions of all actions on cells:
That seems overly verbose. he approach implemented in this PR automatically provides a natural way of have actions that either the selected of mulitple marked cells. |
|
The verbosity will not be annoying for user to type once we have fuzzy-matching, but I understand your point. Though I dont' think verbosity should be scarified on action that will almost never be used through the command palette (run current cell, move cursor up or previous cell). I see you point though, but keep in mind that many user are not native english, and that they will be exposed to english UI, and that while english user will find some action name obvious, it will not be the case for non-english. |
|
@Carreau yes, I get your point about english versus non-english users. It was actually very helpful to have @SylvainCorlay doing this work with us as he had similar feedback as you. One suggestion was to use German for all of these as it is a quite formal language with more rigid and structured grammar rules. But I think |
|
@Carreau yes, I like your idea of being able to hide some actions from the command pallete. We are pretty sure that some of them won't ever be run that way. Pinging @sccolbert @blink1073 on that idea. |
We are all consenting adults. Hide by default action stat start with |
|
+1 on having a prefix to distinguish advanced commands from every-day commands. I might choose |
|
+1 to most of this, I think it's a great improvement. Thanks for doing a thoughtful pass on the names. Only a couple of questions:
|
|
The choice for |
|
+1 for a prefix other than |
|
Following existing convention is a good reason, thanks. |
|
I'm fine with either $ or _ , the old hackpad notes for commands is here |
|
Seem to need rebase (sorry about that) |
|
No problem, I figure I would have to rebase. On Wed, Oct 14, 2015 at 2:13 PM, Matthias Bussonnier <
Brian E. Granger |
f4c6883 to
17b00f2
Compare
|
Rebased @minrk On the verbose But maybe a bigger question. I would like us to start using a unique prefix approach for CSS, etc. Alot of this will start to be scoped at the level of our JS npm packages. For example, it would be great if the command and CSS namespace for the notebook component could be something like What global name prefix to people like? @sccolbert @blink1073 @minrk @Carreau @jdfreder @dwillmer @jasongrout @SylvainCorlay |
|
I prefer |
|
And a notebook file format would be |
|
OK I have rebased, and I think fixed the tests. I think we should wait to add the command hiding in a separate PR. I like the idea of using Remaining things to solve for this:
|
473878a to
4c22335
Compare
|
OK I have addressed most comments. I talked more with Chris, Jason and Sylvain about the action name prefix naming.
Based on these discussions, the current state of the PR is to use As an aside, I think it would be helpful to use a different name (maybe the shorter Thoughts? |
I'm still unconvinced about
The fact that the toolbar is not in env is weird, and yes |
|
Yeah, I am not thrilled about the |
|
Maybe fair enough for notebook, but really why not just |
|
We could, but in the long run we are going to end up with separate JS and It might not be so bad though... On Tue, Oct 20, 2015 at 11:42 AM, Matthias Bussonnier <
Brian E. Granger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jp-notebook here, you are using jupyter-notebook everywhere else.
|
Can you check the pager, click on |
|
And let's merge once the jp-notebook is fixed, and iterate. |
|
OK, after Travis is happy, this is ready to merge. I have also figured out why the pager is broken, but it is also broken in master so I will submit another PR to fix... |
Seem fair enough. Traavis is happy, rereading one last time and I'll press the GB. |
|
Changes Unknown when pulling dab9765 on ellisonbg:action-names into ** on jupyter:master**. |
This PR does a review of the action names in the notebook to address #532.
We sat down with @sccolbert @jasongrout and @SylvainCorlay last week and spent a day going through all the action names, and seeing what patterns and inconsistencies there were. Here are the broad rules we are using for these names:
This PR also starts to use
:for action groups (looking forward to phosphor) andjupyter-notebookas the default group.