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

Use DiffEditor of monaco (fix for #2275 and #2260) #2298

Conversation

mehmetoguzderin
Copy link
Contributor

@mehmetoguzderin mehmetoguzderin commented Sep 25, 2019

Aims to solve #2275 and #2260.

This pull request migrates generation of content for revision-diff from ReactDiffViewer to DiffEditor provided by monaco itself.

revision-diff-content serves as a non-redux wrapper for abstracting details of monaco away. Further connections such as theming should be communicated as properties from revision-diff to revision-diff-content in render function.

@mehmetoguzderin mehmetoguzderin changed the title Use DiffEditor of monaco (fix for #2275) Use DiffEditor of monaco (fix for #2275 and #2260) Sep 25, 2019
mehmetoguzderin and others added 8 commits September 26, 2019 00:37
- add a few default pytest parameters that I use often
- add flake8 linting
- add coverage linting
- add isort linting
- add ability to disable flake8, coverage, isort with new --lean parameter for pytest
- add staticfiles pytest plugin
- restructure tests directory a bit to stop requiring sys.path modification
- fix a warning that showed up because of freezegun
@wlach
Copy link
Contributor

wlach commented Sep 27, 2019

This looks great @mehmetoguzderin! I'm not sure why the unit tests are failing though.

My only other comment is that the initial revision diff looks a little strange (it claims a line is deleted at the top):
image

@mehmetoguzderin
Copy link
Contributor Author

This looks great @mehmetoguzderin! I'm not sure why the unit tests are failing though.

My only other comment is that the initial revision diff looks a little strange (it claims a line is deleted at the top):
image

Thanks!

It is strange indeed and have been noticed by others too:
microsoft/monaco-editor#630

Maybe we can specialize in the initial revision to solve this after we fix the unit tests issue.

renovate bot and others added 11 commits September 28, 2019 14:22
Fixes a console error by implicitly removing the dependency on react-side-effect
(see nfl/react-helmet#465).
…ect#2314)

Some URLs that people use can get quite long, so best to increase
the max amount here.
@wlach
Copy link
Contributor

wlach commented Oct 2, 2019

@mehmetoguzderin do you want to try rebasing this against latest master? The monaco upgrade in #2273 may have either fixed this or made the test error output more intelligible, if we're lucky.

jezdez and others added 28 commits October 2, 2019 19:13
This will let us reach out more easily to people who log into
alpha.iodide.io. Standard Mozilla privacy policy still applies
and GitHub will ensure that people consent to this.
) (iodide-project#2332)

It should not be required for public projects like Iodide. Instead, import enough 
circle environment into our docker compose environment so uploads can
complete successfully.
This reverts commit b9bc6e3.
This reverts commit afb69c4.
This reverts commit 6746cca.
This reverts commit 7a551fc.
This reverts commit 8ceeb6b.
This reverts commit f3438b8, reversing
changes made to 40fa7fc.
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