-
Notifications
You must be signed in to change notification settings - Fork 160
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
[WIP] Fix/improve manual edit #211
Open
vidartf
wants to merge
48
commits into
jupyter:master
Choose a base branch
from
vidartf:manual-edit
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note: This is intended for merge after the next release, as it likely needs more polishing than we are able to perform in time. |
Need to be there for manual edits.
All custom edits gets their own lines, so character highlighting is unnecessary.
- Use actual patched value - Copy over source also if it is not present on patch op itself (typically for pushed patches)
Don't try to replace text if already synced (will lead to invalid value).
Don't apply mixed chunk borders on top of custom chunks
The value of `cm.getValue()` was previously not well-defined if multiple changes had occurred within the same operation (e.g. a newline followed by an automatic indent). By reconstructing ourself, we should be OK as long as model stays in sync!
Assumed not to be critical. If reverting, some of the manual edit alignment might be off, as line chunking becomes unreliable.
Always choose the simplest diff.
If you undo a manual edit by using the pickers, the decision should be deleted unless there is a local/remote diff. If all decisions of a chunk are removed, the picker should be removed as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR should add proper support for manual editing in the merge pane of the merge view.
This isn't completely ready for merge yet, but it is good enough to share, and to recruit help in breaking it in interesting ways. Indicators for breaking: edited text becomes incorrect; an error notification pops up complaining about model/editor out of sync. Alignment issues are not critical for the end saved result, but might indicate that the decisions/diffs are a little wonky (and might therefore be easy to break with subsequent edits).
Currently this is only supported for a four-way merge view, so I also added a "force four way view" button to the header of all cells that are not four-way. In the future this might be solved more elegantly, but this gets the job done.
Fixes #195.