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

Added shortcut for run all cells #4558

Merged
merged 2 commits into from May 15, 2018
Merged

Added shortcut for run all cells #4558

merged 2 commits into from May 15, 2018

Conversation

@palewire
Copy link
Contributor

@palewire palewire commented May 12, 2018

I propose adding this new shortcut: CTRL+SHIFT+ENTER. It will run all cells in the active notebook.

This is something I find myself doing often in my daily use of the notebook. It will be immediately useful to me and I suspect to many others.

"default": { },
"properties": {
"command": { "default": "runmenu:run-all" },
"keys": { "default": ["Ctrl Shift Enter"] },
Copy link
Contributor

@ellisonbg ellisonbg May 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use use Accel rather than Ctrl so it will map to command on a mac.

@palewire
Copy link
Contributor Author

@palewire palewire commented May 12, 2018

No problem, @ellisonbg. I've made that change and pushed it.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented May 12, 2018

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented May 15, 2018

Merging! Many thanks for the contribution!

@ellisonbg ellisonbg merged commit 610a280 into jupyterlab:master May 15, 2018
2 checks passed
@palewire palewire deleted the run-all-shortcuts branch May 15, 2018
@jasongrout jasongrout added this to the Beta 3 milestone Jun 12, 2018
@ian-r-rose ian-r-rose mentioned this pull request Jul 20, 2018
@BoPeng
Copy link
Collaborator

@BoPeng BoPeng commented Jul 31, 2018

@ellisonbg We are discussing possible shortcuts for the run-in-console command in gitter. I suggested C-S-Enter because it is in line with C-Enter and S-Enter and because users tend to use this shortcut repeatedly to step through a cell for debugging, and thenC-Enter or S-Enter to actually execute the cell. I was referred to this PR which assigns the shortcut to execute-all.

I do not think it is a good idea to assign any shortcut to "execute all cells" because it is a major operation that are not used very often, and can be troublesome if triggered accidentally (e.g. when C-S-Enter is entered when messing with C-Enter and S-Enter). In my case with bioinformatic data analysis, "execute all cells" will take very long time to complete and is executed only after everything is done to make sure the notebook is reproducible. Is it possible for us to re-consider this PR?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jul 31, 2018

+1 to reconsidering this keyboard shortcut, for the reasons @BoPeng suggested. The execution item in question is in the menu, so it is still easy to access.

@palewire
Copy link
Contributor Author

@palewire palewire commented Jul 31, 2018

I have not conducted a public opinion poll or anything, but I can tell you with at least some certainty there is backing for making this option easier to access.

I got some good online reaction to the pull request. I've heard from numerous people IRL they are eager to have this option. I even had an old school notebook user in my office tell me the hot key was enough to make him consider upgrading to lab. I personally will use it every day.

While it certainly isn't used by every user, I can assure you that @BoPeng's claim it is "not used very often" is inaccurate. I use "run all cells" multiple times daily, and from talking to my peers in the data journalism community I know it's frequently done to clean out and rerun the small sized notebooks we typically write.

@BoPeng
Copy link
Collaborator

@BoPeng BoPeng commented Jul 31, 2018

IMHO "multiple times daily" is far from "often" to justify a keyboard shortcut, compared to "C-Enter" etc that are used throughout the use of Jupyter, and "Kernel -> Run All" is right there in the menu.

Also, whereas your notebooks might be small enough to re-execute easily, others (like me) might have large notebooks that take very long time to complete so an accidental 'run-all' might force me to restart the kernel and lose hours of work.

@palewire
Copy link
Contributor Author

@palewire palewire commented Jul 31, 2018

IMHO, the solution to that is you simply don't use the hot key. I don't see how anyone is being forced to do anything.

@BoPeng
Copy link
Collaborator

@BoPeng BoPeng commented Jul 31, 2018

Ctrl and Shift are very close on the keyboard and could be pressed together subconsciously, and people might try C-Enter, S-Enter, and C-S-Enter if they are uncertain which shortcut is used to execute a cell. My point here is that any major action that people need to think twice before execution should not have a shortcut.

The tweeter link you cited actually raised another question: why "Run All" instead of "Restart Kernel and Run All" because the latter is cleaner (and is the one I would use)? Should we assign another shortcut? Anyway, I think I have listed all my reasons against this PR and will let others comment.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jul 31, 2018

people might try C-Enter, S-Enter, and C-S-Enter if they are uncertain which shortcuts are used to execute a cell.

That's what I find myself doing when I try to use something other than Shift-Enter.

@palewire
Copy link
Contributor Author

@palewire palewire commented Jul 31, 2018

If you can think of another key combination you'd prefer for run-all-cells, that's fine with me.

But I don't see any strong reason for excluding this hot key that wouldn't also argue for excluding a significant number of other shortcuts that also have a drastic effect on the state of the notebook.

For instance, there are now hot keys to restart and terminate the kernel, which have the most drastic effect I can think of. There are also buttons to do the same thing, which could be clicked "subconsciously." In addition, there is a hotkey that can convert a code cell to a markdown cell, which I believe requires it to be rerun. The list goes on and on.

Now that you mention it, I would welcome adding a restart-and-run-all shortcut. It seems like a logical next step after merging this one. There should be a hotkey for both, IMHO, and virtually everything else in the menus. If it's important enough to be in the menu, it's important enough to have a hot key, IMHO, and I haven't seen a reason to start drawing lines between the two beyond some vague personal preference.

@BoPeng
Copy link
Collaborator

@BoPeng BoPeng commented Jul 31, 2018

The shortcut for restarting the kernel is command mode, 0, 0, which is quite difficult to be mistaken, and there is a confirmation dialog following the shortcut. I would call it a bug if it is "0" without confirmation because it would be too easy to trigger it. "Change cell to markdown" and back does require re-run to get the original output but the kernel is intact and variables are there. I would not call the change dramatic and I even sometimes use this to clear long output of a cell.

I think it is quite clear to everyone that actions with dramatic consequences should be executed with caution (no simple shortcut, with user confirmation etc). The difference here is that you thought that "run all" is an non-intrusive action and I have quite the opposite use case here.

@palewire
Copy link
Contributor Author

@palewire palewire commented Jul 31, 2018

Is there an alternative key combination for this you would consider "difficult to be mistaken?"

@BoPeng
Copy link
Collaborator

@BoPeng BoPeng commented Jul 31, 2018

I personally do not think a default shortcut is needed for this action, and there should be a confirmation box if a shortcut is assigned (but then the convenience of a shortcut is weakened). With the coming shortcut extension that makes it very easy to assign shortcuts to commands, it is fair enough to allow users who work with mostly small notebooks to assign their own shortcut to "Run All" (and/or for "Restart and Run All") to have straight access (no confirmation) to this command.

@palewire
Copy link
Contributor Author

@palewire palewire commented Jul 31, 2018

I think we'll have to respectfully disagree here. I think such a decision would place your personal preferences above the rest of the user base.

The argument you're making could be made for any shortcut you don't use and I urge the maintainers to seek a more comprehensive solution to handling these problems. I also urge them to keep this merge.

@BoPeng
Copy link
Collaborator

@BoPeng BoPeng commented Jul 31, 2018

I have said this is my personal opinion although I will certainly file a bug report if a shortcut is assigned to this command without a confirmation box.

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

Successfully merging this pull request may close these issues.

None yet

4 participants