From 831e7cda9d2008b9570e1e27b140e4242b5e178f Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Mon, 18 Apr 2016 17:44:32 +0200 Subject: [PATCH] Create shortcut editor for the notebook 1) finish the step allowing the use of es6 - this include some tweak to web pack configuration to speed up recompile in watch mode (in particular cache sourcemaps). - enable eslint (error only), on obvious mistakes. - setup babel to compile to es5 as a target. 2) Make the test pass under Casper that does not always have `Function.prototype.bind` defined, which we cannot patch only in the tests. 3) Write an actual shortcut editor that list and allow to modify most of the command mode shortcut. The logic to persist the shortcuts is a bit tricky as there are default keyboard shortcuts, and so when you "unbind" them you need to re-unbind them at next startup. This does not work for a few shortcut for technical reasons: ``, ``, as well as `` and `` which register asynchronously, so are not detected as "default" shortcuts. --- .babelrc | 3 + .eslintignore | 5 + .eslintrc.json | 13 ++ .travis.yml | 1 + notebook/static/base/js/keyboard.js | 35 ++-- notebook/static/notebook/js/actions.js | 6 + notebook/static/notebook/js/main.js | 19 ++ notebook/static/notebook/js/notebook.js | 58 +++--- notebook/static/notebook/js/shortcuteditor.js | 173 ++++++++++++++++++ notebook/static/notebook/less/notebook.less | 16 ++ notebook/static/tree/js/main.js | 19 ++ notebook/tests/base/utils.js | 14 +- package.json | 6 +- setupbase.py | 1 - webpack.config.js | 6 +- 15 files changed, 324 insertions(+), 51 deletions(-) create mode 100644 .babelrc create mode 100644 .eslintignore create mode 100644 .eslintrc.json create mode 100644 notebook/static/notebook/js/shortcuteditor.js diff --git a/.babelrc b/.babelrc new file mode 100644 index 00000000000..cea726b873d --- /dev/null +++ b/.babelrc @@ -0,0 +1,3 @@ +{ + "presets": ["es2015"], +} diff --git a/.eslintignore b/.eslintignore new file mode 100644 index 00000000000..db228f0817e --- /dev/null +++ b/.eslintignore @@ -0,0 +1,5 @@ +*.min.js +*components* +*node_modules* +*built* +*build* diff --git a/.eslintrc.json b/.eslintrc.json new file mode 100644 index 00000000000..594d5954cb3 --- /dev/null +++ b/.eslintrc.json @@ -0,0 +1,13 @@ +{ + "parserOptions": { + "ecmaVersion": 6, + "sourceType": "module" + }, + "rules": { + "semi": 1, + "no-cond-assign": 2, + "no-debugger": 2, + "comma-dangle": 1, + "no-unreachable" : 2 + } +} diff --git a/.travis.yml b/.travis.yml index 1a7c62fec4e..7bd568e93df 100644 --- a/.travis.yml +++ b/.travis.yml @@ -36,6 +36,7 @@ install: script: - 'if [[ $GROUP == js* ]]; then travis_retry python -m notebook.jstest ${GROUP:3}; fi' + - 'if [[ $GROUP == js/notebook ]]; npm run lint; fi' - 'if [[ $GROUP == python ]]; then nosetests -v --with-coverage --cover-package=notebook notebook; fi' matrix: diff --git a/notebook/static/base/js/keyboard.js b/notebook/static/base/js/keyboard.js index 5c3f3d2b02e..a939e1315aa 100644 --- a/notebook/static/base/js/keyboard.js +++ b/notebook/static/base/js/keyboard.js @@ -231,10 +231,21 @@ define([ } return dct; }; + + + ShortcutManager.prototype.get_action_shortcuts = function(name){ + var ftree = flatten_shorttree(this._shortcuts); + var res = []; + for (var sht in ftree ){ + if(ftree[sht] === name){ + res.push(sht); + } + } + return res; + }; ShortcutManager.prototype.get_action_shortcut = function(name){ var ftree = flatten_shorttree(this._shortcuts); - var res = {}; for (var sht in ftree ){ if(ftree[sht] === name){ return sht; @@ -405,25 +416,27 @@ define([ shortcut = shortcut.toLowerCase(); this.remove_shortcut(shortcut); - var patch = {keys:{}}; - var b = {bind:{}}; + const patch = {keys:{}}; + const b = {bind:{}}; patch.keys[this._mode] = {bind:{}}; patch.keys[this._mode].bind[shortcut] = null; this._config.update(patch); // if the shortcut we unbind is a default one, we add it to the list of // things to unbind at startup + if( this._defaults_bindings.indexOf(shortcut) !== -1 ){ + const cnf = (this._config.data.keys||{})[this._mode]; + const unbind_array = cnf.unbind||[]; - if(this._defaults_bindings.indexOf(shortcut) !== -1){ - var cnf = (this._config.data.keys||{})[this._mode]; - var unbind_array = cnf.unbind||[]; // unless it's already there (like if we have remapped a default - // shortcut to another command, and unbind it) - if(unbind_array.indexOf(shortcut) !== -1){ - unbind_array.concat(shortcut); - var unbind_patch = {keys:{unbind:unbind_array}}; - this._config._update(unbind_patch); + // shortcut to another command): unbind it) + if(unbind_array.indexOf(shortcut) === -1){ + const _parray = unbind_array.concat(shortcut); + const unbind_patch = {keys:{}}; + unbind_patch.keys[this._mode] = {unbind:_parray} + console.warn('up:', unbind_patch); + this._config.update(unbind_patch); } } }; diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index 83df33beebf..6016756769f 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -62,6 +62,12 @@ define(function(require){ * **/ var _actions = { + 'edit-command-mode-keyboard-shortcuts': { + help: 'Open a dialog to edit the command mode keyboard shortcuts', + handler: function (env) { + env.notebook.show_shortcuts_editor(); + } + }, 'restart-kernel': { help: 'restart the kernel (no confirmation dialog)', handler: function (env) { diff --git a/notebook/static/notebook/js/main.js b/notebook/static/notebook/js/main.js index c5030d42b2e..e4ffae1357c 100644 --- a/notebook/static/notebook/js/main.js +++ b/notebook/static/notebook/js/main.js @@ -2,6 +2,25 @@ // Distributed under the terms of the Modified BSD License. __webpack_public_path__ = window['staticURL'] + 'notebook/js/built/'; +// adapted from Mozilla Developer Network example at +// https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Function/bind +// shim `bind` for testing under casper.js +var bind = function bind(obj) { + var slice = [].slice; + var args = slice.call(arguments, 1), + self = this, + nop = function() { + }, + bound = function() { + return self.apply(this instanceof nop ? this : (obj || {}), args.concat(slice.call(arguments))); + }; + nop.prototype = this.prototype || {}; // Firefox cries sometimes if prototype is undefined + bound.prototype = new nop(); + return bound; +}; +Function.prototype.bind = Function.prototype.bind || bind ; + + requirejs(['contents'], function(contentsModule) { require([ 'base/js/namespace', diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index df9a23825f9..9f0404c7928 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -4,31 +4,31 @@ /** * @module notebook */ -define(function (require) { - "use strict"; - var IPython = require('base/js/namespace'); - var _ = require('underscore'); - var utils = require('base/js/utils'); - var dialog = require('base/js/dialog'); - var cellmod = require('notebook/js/cell'); - var textcell = require('notebook/js/textcell'); - var codecell = require('notebook/js/codecell'); - var moment = require('moment'); - var configmod = require('services/config'); - var session = require('services/sessions/session'); - var celltoolbar = require('notebook/js/celltoolbar'); - var marked = require('components/marked/lib/marked'); - var CodeMirror = require('codemirror/lib/codemirror'); - var runMode = require('codemirror/addon/runmode/runmode'); - var mathjaxutils = require('notebook/js/mathjaxutils'); - var keyboard = require('base/js/keyboard'); - var tooltip = require('notebook/js/tooltip'); - var default_celltoolbar = require('notebook/js/celltoolbarpresets/default'); - var rawcell_celltoolbar = require('notebook/js/celltoolbarpresets/rawcell'); - var slideshow_celltoolbar = require('notebook/js/celltoolbarpresets/slideshow'); - var attachments_celltoolbar = require('notebook/js/celltoolbarpresets/attachments'); - var scrollmanager = require('notebook/js/scrollmanager'); - var commandpalette = require('notebook/js/commandpalette'); +"use strict"; +import IPython from 'base/js/namespace'; +import _ from 'underscore'; +import utils from 'base/js/utils'; +import dialog from 'base/js/dialog'; +import cellmod from 'notebook/js/cell'; +import textcell from 'notebook/js/textcell'; +import codecell from 'notebook/js/codecell'; +import moment from 'moment'; +import configmod from 'services/config'; +import session from 'services/sessions/session'; +import celltoolbar from 'notebook/js/celltoolbar'; +import marked from 'components/marked/lib/marked'; +import CodeMirror from 'codemirror/lib/codemirror'; +import runMode from 'codemirror/addon/runmode/runmode'; +import mathjaxutils from 'notebook/js/mathjaxutils'; +import keyboard from 'base/js/keyboard'; +import tooltip from 'notebook/js/tooltip'; +import default_celltoolbar from 'notebook/js/celltoolbarpresets/default'; +import rawcell_celltoolbar from 'notebook/js/celltoolbarpresets/rawcell'; +import slideshow_celltoolbar from 'notebook/js/celltoolbarpresets/slideshow'; +import attachments_celltoolbar from 'notebook/js/celltoolbarpresets/attachments'; +import scrollmanager from 'notebook/js/scrollmanager'; +import commandpalette from 'notebook/js/commandpalette'; +import {ShortcutEditor} from 'notebook/js/shortcuteditor'; var _SOFT_SELECTION_CLASS = 'jupyter-soft-selected'; @@ -50,7 +50,7 @@ define(function (require) { * @param {string} options.notebook_path * @param {string} options.notebook_name */ - var Notebook = function (selector, options) { + export function Notebook(selector, options) { this.config = options.config; this.class_config = new configmod.ConfigWithDefaults(this.config, Notebook.options_default, 'Notebook'); @@ -362,6 +362,10 @@ define(function (require) { var x = new commandpalette.CommandPalette(this); }; + Notebook.prototype.show_shortcuts_editor = function() { + new ShortcutEditor(this); + }; + /** * Trigger a warning dialog about missing functionality from newer minor versions */ @@ -3160,5 +3164,3 @@ define(function (require) { this.load_notebook(this.notebook_path); }; - return {'Notebook': Notebook}; -}); diff --git a/notebook/static/notebook/js/shortcuteditor.js b/notebook/static/notebook/js/shortcuteditor.js new file mode 100644 index 00000000000..0d59c82c773 --- /dev/null +++ b/notebook/static/notebook/js/shortcuteditor.js @@ -0,0 +1,173 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + + +import QH from "notebook/js/quickhelp"; +import dialog from "base/js/dialog"; +import {render} from "preact"; +import {createElement, createClass} from "preact-compat"; +import marked from 'components/marked/lib/marked'; + +/** + * Humanize the action name to be consumed by user. + * internally the actions name are of the form + * : + * we drop and replace dashes for space. + */ +const humanize_action_id = function(str) { + return str.split(':')[1].replace(/-/g, ' ').replace(/_/g, '-'); +}; + +/** + * given an action id return 'command-shortcut', 'edit-shortcut' or 'no-shortcut' + * for the action. This allows us to tag UI in order to visually distinguish + * Wether an action have a keybinding or not. + **/ + +const KeyBinding = createClass({ + displayName: 'KeyBindings', + getInitialState: function() { + return {shrt:''}; + }, + handleShrtChange: function (element){ + this.setState({shrt:element.target.value}); + }, + render: function(){ + const that = this; + const available = this.props.available(this.state.shrt); + const empty = (this.state.shrt === ''); + return createElement('div', {className:'jupyter-keybindings'}, + createElement('i', {className: "pull-right fa fa-plus", alt: 'add-keyboard-shortcut', + onClick:()=>{ + available?that.props.onAddBindings(that.state.shrt, that.props.ckey):null; + that.state.shrt=''; + } + }), + createElement('input', { + type:'text', + placeholder:'add shortcut', + className:'pull-right'+((available||empty)?'':' alert alert-danger'), + value:this.state.shrt, + onChange:this.handleShrtChange + }), + this.props.shortcuts?this.props.shortcuts.map((item, index) => { + return createElement('span', {className: 'pull-right'}, + createElement('kbd', {}, [ + item.h, + createElement('i', {className: "fa fa-times", alt: 'remove '+item.h, + onClick:()=>{ + that.props.unbind(item.raw); + } + }) + ]) + ); + }):null, + createElement('div', {title: '(' +this.props.ckey + ')' , className:'jupyter-keybindings-text'}, this.props.display ) + ); + } +}); + +const KeyBindingList = createClass({ + displayName: 'KeyBindingList', + getInitialState: function(){ + return {data:[]}; + }, + componentDidMount: function(){ + this.setState({data:this.props.callback()}); + }, + render: function() { + const childrens = this.state.data.map((binding)=>{ + return createElement(KeyBinding, Object.assign({}, binding, {onAddBindings:(shortcut, action)=>{ + this.props.bind(shortcut, action); + this.setState({data:this.props.callback()}); + }, + available:this.props.available, + unbind: (shortcut) => { + this.props.unbind(shortcut); + this.setState({data:this.props.callback()}); + } + })); + }); + childrens.unshift(createElement('div', {className:'well', key:'disclamer', dangerouslySetInnerHTML: + {__html: + marked( + + "This dialog allows you to modify the keymap of the command mode, and persist the changes. "+ + "You can define many type of shorctuts and sequences of keys. "+ + "\n\n"+ + " - Use dashes `-` to represent keys that should be pressed with modifiers, "+ + "for example `Shift-a`, or `Ctrl-;`. \n"+ + " - Separate by commas if the keys need to be pressed in sequence: `h,a,l,t`.\n"+ + "\n\nYou can combine the two: `Ctrl-x,Meta-c,Meta-b,u,t,t,e,r,f,l,y`.\n"+ + "Casing will have no effects: (e.g: `;` and `:` are the same on english keyboards)."+ + " You need to explicitelty write the `Shift` modifier.\n"+ + "Valid modifiers are `Cmd`, `Ctrl`, `Alt` ,`Meta`, `Cmdtrl`. Refer to developper docs "+ + "for their signification depending on the platform." + )} + })); + return createElement('div',{}, childrens); + } +}); + +const get_shortcuts_data = function(notebook) { + const actions = Object.keys(notebook.keyboard_manager.actions._actions); + const src = []; + + for (let i = 0; i < actions.length; i++) { + const action_id = actions[i]; + const action = notebook.keyboard_manager.actions.get(action_id); + + let shortcuts = notebook.keyboard_manager.command_shortcuts.get_action_shortcuts(action_id); + let hshortcuts; + if (shortcuts.length > 0) { + hshortcuts = shortcuts.map((raw)=>{ + return {h:QH._humanize_sequence(raw),raw:raw};} + ); + } + src.push({ + display: humanize_action_id(action_id), + shortcuts: hshortcuts, + key:action_id, // react specific thing + ckey: action_id + }); + } + return src; +}; + + +export const ShortcutEditor = function(notebook) { + + if(!notebook){ + throw new Error("CommandPalette takes a notebook non-null mandatory arguement"); + } + + const body = $('
'); + const mod = dialog.modal({ + notebook: notebook, + keyboard_manager: notebook.keyboard_manager, + title : "Edit Command mode Shortcuts", + body : body, + buttons : { + OK : {} + } + }); + + const src = get_shortcuts_data(notebook); + + mod.addClass("modal_stretch"); + + mod.modal('show'); + render( + createElement(KeyBindingList, { + callback:()=>{ return get_shortcuts_data(notebook);}, + bind: (shortcut, command) => { + return notebook.keyboard_manager.command_shortcuts._persist_shortcut(shortcut, command); + }, + unbind: (shortcut) => { + return notebook.keyboard_manager.command_shortcuts._persist_remove_shortcut(shortcut); + }, + available: (shrt) => { return notebook.keyboard_manager.command_shortcuts.is_available_shortcut(shrt);} + }), + body.get(0) + ); +}; diff --git a/notebook/static/notebook/less/notebook.less b/notebook/static/notebook/less/notebook.less index 2c3e8e6a3ff..579cdbc2689 100644 --- a/notebook/static/notebook/less/notebook.less +++ b/notebook/static/notebook/less/notebook.less @@ -99,3 +99,19 @@ kbd { padding-top: 1px; padding-bottom: 1px; } + +.jupyter-keybindings { + padding: 0px; + line-height: 24px; + border-bottom: 1px solid gray; +} + +.jupyter-keybindings input { + margin: 0; + padding: 0; + border: none; +} + +.jupyter-keybindings i { + padding: 6px; +} diff --git a/notebook/static/tree/js/main.js b/notebook/static/tree/js/main.js index 5d9eae96dc6..9ae123c7a17 100644 --- a/notebook/static/tree/js/main.js +++ b/notebook/static/tree/js/main.js @@ -2,6 +2,25 @@ // Distributed under the terms of the Modified BSD License. __webpack_public_path__ = window['staticURL'] + 'tree/js/built/'; +// adapted from Mozilla Developer Network example at +// https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Function/bind +// shim `bind` for testing under casper.js +var bind = function bind(obj) { + var slice = [].slice; + var args = slice.call(arguments, 1), + self = this, + nop = function() { + }, + bound = function() { + return self.apply(this instanceof nop ? this : (obj || {}), args.concat(slice.call(arguments))); + }; + nop.prototype = this.prototype || {}; // Firefox cries sometimes if prototype is undefined + bound.prototype = new nop(); + return bound; +}; +Function.prototype.bind = Function.prototype.bind || bind ; + + requirejs(['contents'], function(contents_service) { require([ 'base/js/namespace', diff --git a/notebook/tests/base/utils.js b/notebook/tests/base/utils.js index f7ba750fba8..e772e5eb7d3 100644 --- a/notebook/tests/base/utils.js +++ b/notebook/tests/base/utils.js @@ -1,14 +1,14 @@ casper.notebook_test(function () { - // Note, \033 is the octal notation of \u001b + // Note, \u001b is the unicode notation of octal \033 which is not officially in js var input = [ - "\033[0m[\033[0minfo\033[0m] \033[0mtext\033[0m", - "\033[0m[\033[33mwarn\033[0m] \033[0m\tmore text\033[0m", - "\033[0m[\033[33mwarn\033[0m] \033[0m https://some/url/to/a/file.ext\033[0m", - "\033[0m[\033[31merror\033[0m] \033[0m\033[0m", - "\033[0m[\033[31merror\033[0m] \033[0m\teven more text\033[0m", + "\u001b[0m[\u001b[0minfo\u001b[0m] \u001b[0mtext\u001b[0m", + "\u001b[0m[\u001b[33mwarn\u001b[0m] \u001b[0m\tmore text\u001b[0m", + "\u001b[0m[\u001b[33mwarn\u001b[0m] \u001b[0m https://some/url/to/a/file.ext\u001b[0m", + "\u001b[0m[\u001b[31merror\u001b[0m] \u001b[0m\u001b[0m", + "\u001b[0m[\u001b[31merror\u001b[0m] \u001b[0m\teven more text\u001b[0m", "\u001b[?25hBuilding wheels for collected packages: scipy", "\x1b[38;5;28;01mtry\x1b[39;00m", - "\033[0m[\033[31merror\033[0m] \033[0m\t\tand more more text\033[0m", + "\u001b[0m[\u001b[31merror\u001b[0m] \u001b[0m\t\tand more more text\u001b[0m", "normal\x1b[43myellowbg\x1b[35mmagentafg\x1b[1mbold\x1b[49mdefaultbg\x1b[39mdefaultfg\x1b[22mnormal", ].join("\n"); diff --git a/package.json b/package.json index e8535c2190e..4235a3c7f5e 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "url": "https://github.com/jupyter/notebook.git" }, "scripts": { + "lint" : "eslint --quiet notebook/", "bower": "bower install --allow-root --config.interactive=false", "build:watch": "concurrent \"npm run build:css:watch\" \"npm run build:js:watch\"", "build": "npm run build:css && npm run build:js", @@ -18,6 +19,7 @@ "build:js:watch": "npm run build:js -- --watch" }, "devDependencies": { + "babel-cli": "^6.7.5", "babel-core": "^6.7.4", "babel-loader": "^6.2.4", "babel-preset-es2015": "^6.6.0", @@ -29,6 +31,8 @@ "webpack": "^1.12.13" }, "dependencies": { - "moment": "^2.8.4" + "moment": "^2.8.4", + "preact": "^4.5.1", + "preact-compat": "^1.7.0" } } diff --git a/setupbase.py b/setupbase.py index 704d7f73f28..548fcb76264 100644 --- a/setupbase.py +++ b/setupbase.py @@ -21,7 +21,6 @@ from distutils.cmd import Command from fnmatch import fnmatch from glob import glob -from multiprocessing.pool import ThreadPool from subprocess import check_call, check_output if sys.platform == 'win32': diff --git a/webpack.config.js b/webpack.config.js index 56e0f53a080..29a068eae30 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -44,7 +44,7 @@ function buildConfig(appName) { filename: 'main.min.js', path: './notebook/static/' + appName + '/js/built' }, - devtool: 'source-map', + devtool: 'eval-source-map', }); } @@ -61,7 +61,7 @@ module.exports = [ path: './notebook/static/services/built', libraryTarget: 'amd' }, - devtool: 'source-map', + devtool: 'eval-source-map', }), _.extend({}, commonConfig, { entry: './notebook/static/index.js', @@ -70,6 +70,6 @@ module.exports = [ path: './notebook/static/built', libraryTarget: 'amd' }, - devtool: 'source-map', + devtool: 'eval-source-map', }), ].map(buildConfig);