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 a shortcut to go to the last run or current running cell #7551

Merged
merged 4 commits into from Nov 27, 2019

Conversation

mlucool
Copy link
Contributor

@mlucool mlucool commented Nov 21, 2019

Code changes

Add a new action and shortcut. By default this shortcut has no hotkeys. I have found that Alt L was the best available option.

User-facing changes

User can add a new hotkey in the settings editor to enable this. The notebook must have execution time toggled on.

@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented Nov 21, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

Copy link
Member

@vidartf vidartf left a comment

Thanks! Looks good, but two minor points to consider.

packages/notebook/src/actions.tsx Outdated Show resolved Hide resolved
if (
execution &&
typeof execution === 'object' &&
'iopub.status.busy' in execution
Copy link
Member

@vidartf vidartf Nov 26, 2019

Choose a reason for hiding this comment

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

Nitpick: Could these two lines be replaced by execution['iopub.status.busy'] !== undefined ?

Copy link
Contributor Author

@mlucool mlucool Nov 27, 2019

Choose a reason for hiding this comment

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

This was done. I will note that I had to cast it to a JSONObject before which is slightly less safe. If execution was set to 3 this will now throw.

Copy link
Member

@vidartf vidartf Nov 27, 2019

Choose a reason for hiding this comment

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

Good point. I think that using @phosphor/coreutils/JSONExt.isObject() will likely be the most robust method here (both in JS and for typings).

For reference, typeof null === 'object' 😕

Copy link
Member

@vidartf vidartf Nov 27, 2019

Choose a reason for hiding this comment

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

(Note: I'm not implying that the previous code was wrong, just that typeof X === 'object' is so counter-intuitive most of the time that I've made it a rule of thumb to simply avoid it)

Copy link
Contributor Author

@mlucool mlucool Nov 27, 2019

Choose a reason for hiding this comment

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

Fixed this anyway. Sadly, isObject fails to return a falsy value with JSONExt.isObject(undefined)

@mlucool
Copy link
Contributor Author

@mlucool mlucool commented Nov 27, 2019

@vidartf all issues have been addressed

@vidartf
Copy link
Member

@vidartf vidartf commented Nov 27, 2019

@mlucool I added another comment above. Kind of nit-picky, so feel free to ignore. Will merge once I've had a look at the CI failures.

@telamonian
Copy link
Member

@telamonian telamonian commented Nov 27, 2019

The Integrity CI is failing cuz of a failed lint check:

++ jlpm run lint:check
yarn run v1.15.2
$ jlpm run prettier:check && jlpm run eslint:check && jlpm run tslint:check
$ prettier --list-different '**/*{.ts,.tsx,.js,.jsx,.css,.json,.md}'
packages/notebook-extension/src/index.ts
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
++ echo 'Please run `jlpm run lint` locally and push changes'
++ exit 1
Please run `jlpm run lint` locally and push changes

@mlucool Did you disable the normal jlab lint-on-commit/push git hooks? I know they give some devs trouble. In any case, you can prob fix the issue by doing what the error says and running:

jlpm run lint

in your repo before commiting/pushing again.

@telamonian telamonian added this to the 2.0 milestone Nov 27, 2019
@telamonian telamonian merged commit 1d787db into jupyterlab:master Nov 27, 2019
10 checks passed
@lock lock bot added the status:resolved-locked label Dec 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement pkg:notebook status:resolved-locked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants