Skip to content

Conversation

@timnlupo
Copy link

@timnlupo timnlupo commented Aug 3, 2019

This change introduces the feature to be able to view plain text diffs. It uses Monaco to render the diffs for supported files and ties into the existing component from visual diffs for Notebooks.

@dhirschfeld
Copy link
Member

Does this resolve #252?

@fcollonval
Copy link
Member

Thanks @timnlupo for this great work.

Your current PR highlights one point I was fearing when using Monaco: the bundle size. From the travis build, it looks we may load more than 10MiB to show some diffs (see below). Would it be possible to include only the editor and not the language workers? It seems it is the default according to https://github.com/jupyterlab/jupyterlab-git/pull/388/files#diff-b7a2b01fa406ddc8bfdcf38ea0eaf567R32.

                                                            Asset      Size         Chunks             Chunk Names
       JUPYTERLAB_FILE_LOADER_jupyterlab-git-css.worker.bundle.js  1.57 MiB     css.worker  [emitted]  css.worker
   JUPYTERLAB_FILE_LOADER_jupyterlab-git-css.worker.bundle.js.map  1.68 MiB     css.worker  [emitted]  css.worker
    JUPYTERLAB_FILE_LOADER_jupyterlab-git-editor.worker.bundle.js   372 KiB  editor.worker  [emitted]  editor.worker
JUPYTERLAB_FILE_LOADER_jupyterlab-git-editor.worker.bundle.js.map   379 KiB  editor.worker  [emitted]  editor.worker
      JUPYTERLAB_FILE_LOADER_jupyterlab-git-html.worker.bundle.js  1.02 MiB    html.worker  [emitted]  html.worker
  JUPYTERLAB_FILE_LOADER_jupyterlab-git-html.worker.bundle.js.map  1.09 MiB    html.worker  [emitted]  html.worker
      JUPYTERLAB_FILE_LOADER_jupyterlab-git-json.worker.bundle.js   745 KiB    json.worker  [emitted]  json.worker
  JUPYTERLAB_FILE_LOADER_jupyterlab-git-json.worker.bundle.js.map   753 KiB    json.worker  [emitted]  json.worker
        JUPYTERLAB_FILE_LOADER_jupyterlab-git-ts.worker.bundle.js  8.27 MiB      ts.worker  [emitted]  ts.worker
    JUPYTERLAB_FILE_LOADER_jupyterlab-git-ts.worker.bundle.js.map  9.22 MiB      ts.worker  [emitted]  ts.worker

@jaipreet-s
Copy link
Member

Hi @fcollonval , @timnlupo was doing this work as part of his internship w/ my team. His internship is over so this work is stalled now.

Would you like to take over this work and drive it forward? If so, it would be fine to take Tim's code as a baseline and then build on top of it.

@fcollonval
Copy link
Member

@jaipreet-s, I gladly push this forward.

@timnlupo
Copy link
Author

Hi @fcollonval , thanks for taking this over. I do believe you can only include the editor worker. From this issue:

"The webworkers are responsible for enhanced suggestions and syntax errors etcs. With setting the editor language to JSON everything is going to be loaded, not only the colors."

Since the diff viewer is read only, I don't believe those features are necessary. Let me know if you have any other questions.

@fcollonval fcollonval mentioned this pull request Sep 7, 2019
3 tasks
@fcollonval
Copy link
Member

Superseeded by #406

@fcollonval fcollonval closed this Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants