Merge pull request #515 from vidartf/cleanup

Cleanup and lab 2.0 fixes

.travis.yml

@@ -22,6 +22,7 @@ cache:
     - nbdime-web/node_modules # NPM packages
     - $HOME/.npm
 before_install:
+  - python -c "import fcntl; fcntl.fcntl(1, fcntl.F_SETFL, 0)"
   - pip install -U pip setuptools
   - nvm install 8
   - |

appveyor.yml

@@ -9,6 +9,10 @@ environment:
       PYTHON_VERSION: "3.5.x"
       PYTHON_MAJOR: 3
       PYTHON_ARCH: "64"
+    - PYTHON: "C:\\Python38-x64"
+      PYTHON_VERSION: "3.8.x"
+      PYTHON_MAJOR: 3
+      PYTHON_ARCH: "64"
 
 # build cache to preserve files/folders between builds
 cache:

docs/source/installing.rst

@@ -49,7 +49,7 @@ and nbdime's web-based viewers depend on the following Node.js packages:
   - json-stable-stringify
   - jupyter-js-services
   - jupyterlab
-  - phosphor
+  - lumino
 
 
 Installing latest development version

nbdime/tests/conftest.py

@@ -287,6 +287,11 @@ def merge_validator(request, json_schema_merge):
             json_schema_merge),
     )
 
+@fixture(scope='session')
+def ioloop_patch():
+    from nbdime.webapp.nbdimeserver import asyncio_patch
+    asyncio_patch()
+
 
 class NBTestDataBase(object):
     def __init__(self):

nbdime/tests/test_git_diffdriver.py

@@ -355,7 +355,7 @@ def test_git_diff_driver_no_filter_without_flag(git_repo, filespath, capsys):
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
-def test_git_web_diff_driver(filespath, unique_port, reset_log):
+def test_git_web_diff_driver(filespath, unique_port, reset_log, ioloop_patch):
     # Simulate a call from `git diff` to check basic driver functionality
 
     fn1 = os.path.join(filespath, 'foo--1.ipynb')

nbdime/tests/test_git_filter_integration.py

@@ -2,6 +2,7 @@
 import io
 import os
 from six import StringIO
+from subprocess import CalledProcessError
 
 import nbformat
 
@@ -97,11 +98,16 @@ def test_apply_filter_invalid_filter(git_repo):
 
 
 def test_apply_filter_valid_filter(git_repo):
+    try:
+        call('cat --help')
+        filter_cmd = 'cat'
+    except (CalledProcessError, FileNotFoundError):
+        filter_cmd = 'findstr x*'
     path = pjoin(git_repo, 'diff.ipynb')
     gitattr = locate_gitattributes()
     with io.open(gitattr, 'a', encoding="utf8") as f:
         f.write(u'\n*.ipynb\tfilter=myfilter\n')
-    call('git config --local --add filter.myfilter.clean "cat"')
+    call('git config --local --add filter.myfilter.clean "%s"' % filter_cmd)
     f = apply_possible_filter(path)
     assert isinstance(f, StringIO)
     # Read validates notebook:

nbdime/tests/test_web.py

@@ -27,7 +27,7 @@
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
-def test_diff_web(filespath, unique_port, reset_log):
+def test_diff_web(filespath, unique_port, reset_log, ioloop_patch):
     a = os.path.join(filespath, diff_a)
     b = os.path.join(filespath, diff_b)
     loop = ioloop.IOLoop.current()
@@ -36,7 +36,7 @@ def test_diff_web(filespath, unique_port, reset_log):
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
-def test_diff_web_localhost(filespath, unique_port, reset_log):
+def test_diff_web_localhost(filespath, unique_port, reset_log, ioloop_patch):
     a = os.path.join(filespath, diff_a)
     b = os.path.join(filespath, diff_b)
     loop = ioloop.IOLoop.current()
@@ -51,7 +51,7 @@ def test_diff_web_localhost(filespath, unique_port, reset_log):
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
-def test_diff_web_gitrefs(git_repo2, unique_port, reset_log):
+def test_diff_web_gitrefs(git_repo2, unique_port, reset_log, ioloop_patch):
     a = 'local'
     b = 'remote'
     c = 'diff.ipynb'
@@ -62,7 +62,7 @@ def test_diff_web_gitrefs(git_repo2, unique_port, reset_log):
 
 
 @pytest.mark.timeout(timeout=WEB_TEST_TIMEOUT)
-def test_merge_web(filespath, unique_port, reset_log):
+def test_merge_web(filespath, unique_port, reset_log, ioloop_patch):
     a = os.path.join(filespath, merge_a)
     b = os.path.join(filespath, merge_b)
     c = os.path.join(filespath, merge_c)

nbdime/webapp/nbdimeserver.py

@@ -101,11 +101,11 @@ def read_notebook(self, arg):
                 nb = nbformat.reads(r.text, as_version=4)
         except requests.exceptions.HTTPError as e:
             self.log.exception(e)
-            raise web.HTTPError(422, 'Invalid notebook: %s, received http error: %s' % (arg, string(e)))
+            raise web.HTTPError(422, 'Invalid notebook: %s, received http error: %s' % (arg, str(e)))
         except Exception as e:
             self.log.exception(e)
             raise web.HTTPError(422, 'Invalid notebook: %s' % arg)
-    
+
         return nb
 
     def get_notebook_argument(self, argname):
@@ -304,6 +304,39 @@ def post(self):
         ioloop.IOLoop.current().stop()
 
 
+def asyncio_patch():
+    """set default asyncio policy to be compatible with tornado
+
+    Tornado 6 (at least) is not compatible with the default
+    asyncio implementation on Windows
+
+    Pick the older SelectorEventLoopPolicy on Windows
+    if the known-incompatible default policy is in use.
+
+    do this as early as possible to make it a low priority and overrideable
+
+    ref: https://github.com/tornadoweb/tornado/issues/2608
+
+    FIXME: if/when tornado supports the defaults in asyncio,
+            remove and bump tornado requirement for py38
+    """
+    if sys.platform.startswith("win") and sys.version_info >= (3, 8):
+        import asyncio
+        try:
+            from asyncio import (
+                WindowsProactorEventLoopPolicy,
+                WindowsSelectorEventLoopPolicy,
+            )
+        except ImportError:
+            pass
+            # not affected
+        else:
+            if type(asyncio.get_event_loop_policy()) is WindowsProactorEventLoopPolicy:
+                # WindowsProactorEventLoopPolicy is not compatible with tornado 6
+                # fallback to the pre-3.8 default of Selector
+                asyncio.set_event_loop_policy(WindowsSelectorEventLoopPolicy())
+
+
 def make_app(**params):
     base_url = params.pop('base_url', '/')
     handlers = [
@@ -355,8 +388,8 @@ def make_app(**params):
     app.exit_code = 0
     return app
 
-
 def init_app(on_port=None, closable=False, **params):
+    asyncio_patch()
     _logger.debug('Using params: %s', params)
     params.update({'closable': closable})
     port = params.pop('port', 0)

packages/nbdime/src/common/collapsible.css

@@ -4,20 +4,14 @@
 }
 
 button.jp-CollapsiblePanel-header-icon {
-    background-size: 20px;
-    background-position: center;
-    background-repeat: no-repeat;
-
     height: 24px;
     width: 24px;
     float: right;
     font-size: var(--jp-ui-font-size1);
-    line-height: var(--jp-private-toolbar-height);
     margin-top: .1em;
-    font-weight: 500;
-    padding: var(--jp-flat-button-padding);
+    padding: 0 2px;
 
-    color: var(--jp-ui-font-color1);
+    color: var(--jp-ui-font-color2);
     background-color: transparent;
 
     border: 1px solid var(--jp-nbdime-output-color2);
@@ -39,14 +33,6 @@ button.jp-CollapsiblePanel-header-icon:active {
     border: 1px solid var(--jp-ui-font-color1);
 }
 
-.jp-CollapsiblePanel-header-icon-opened {
-    background-image: var(--jp-icon-caretup);
-}
-
-.jp-CollapsiblePanel-header-icon-closed {
-    background-image: var(--jp-icon-caretdown);
-}
-
 .jp-CollapsiblePanel-header {
     padding-top: .2em;
     padding-bottom: .2em;

packages/nbdime/src/common/collapsiblepanel.ts

@@ -63,6 +63,11 @@ class CollapsiblePanel extends Panel {
       collapsed === true ?
       COLLAPSIBLE_HEADER_ICON_CLOSED :
       COLLAPSIBLE_HEADER_ICON_OPEN);
+    this.button.classList.add("fa");
+    this.button.classList.add(
+      collapsed === true ?
+      "fa-caret-down" :
+      "fa-caret-up");
   }
 
   toggleCollapsed(): void {
@@ -73,12 +78,16 @@ class CollapsiblePanel extends Panel {
       slider.addClass(COLLAPSIBLE_OPEN);
       button.classList.remove(COLLAPSIBLE_HEADER_ICON_CLOSED);
       button.classList.add(COLLAPSIBLE_HEADER_ICON_OPEN);
+      this.button.classList.remove("fa-caret-down");
+      this.button.classList.add("fa-caret-up");
 
     } else {
       slider.removeClass(COLLAPSIBLE_OPEN);
       slider.addClass(COLLAPSIBLE_CLOSED);
       button.classList.remove(COLLAPSIBLE_HEADER_ICON_OPEN);
       button.classList.add(COLLAPSIBLE_HEADER_ICON_CLOSED);
+      this.button.classList.remove("fa-caret-up");
+      this.button.classList.add("fa-caret-down");
     }
   }
 

packages/nbdime/src/patch/common.ts

@@ -81,7 +81,7 @@ class PatchObjectHelper implements IIterator<string> {
     return this;
   }
 
-  next(): string {
+  next(): string | undefined {
     let key = this._remainingKeys.shift();
     if (key && valueIn(key, this._diffKeys)) {
       let op = this._diffLUT[key].op;
@@ -93,8 +93,7 @@ class PatchObjectHelper implements IIterator<string> {
         this._currentIsAddition = undefined;
       }
     }
-    // Cast as non-undefined as phosphor doesn't support strict null checks yet:
-    return key!;
+    return key;
   }
 
   clone(): IIterator<string> {