Fix Syntax highlight for token change in added/deleted code #12238
Conversation
This file contains hidden or 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
For diffs we first syntax highlight the before and after line then use a 3rd party diff library to find the difference in them and create a substring based on that, which we then highlight a 2nd time to show the specific difference within a line that has changed. In a specific case if the diffrence also changes the chroma class it will split in the middle of the attr and cause broken HTML: ``` <span class="nx">oldtext<span> <span class="k">var newtext<span> ``` Will then split on ``` <span class=" ``` Where the difference starts, and produce something broken like: ``` <span class="<span class="removed-code">nx"oldtext</span></span ``` Fix that by detecting when it happens and putting the HTML back together properly before highlighting the added/deleted code. Fixes go-gitea#12235
lafriks
added
the
skip-changelog
|
Not totally resolved it seems. Check line 518,519,707,708,709 of this commit.
|
GiteaBot
added
the
lgtm/need 2
|
Ah I see same problem can happen for any diff section not just equal part -- will fix |
…starts with a closing span
|
Think it should work OK now. Thanks for finding all these broken cases |
silverwind
approved these changes
Jul 14, 2020
GiteaBot
added
lgtm/need 1
lafriks
approved these changes
Jul 16, 2020
GiteaBot
added
lgtm/done
mrsdizzie
added a commit
to mrsdizzie/gitea
that referenced
this pull request
Aug 7, 2020
Make previous fix from go-gitea#12238 more robust since I saw a case where a diff changes only a single character in a chroma class instead of the entire thing. Add another more complicated test to match.
zeripath
pushed a commit
that referenced
this pull request
Aug 8, 2020
Make previous fix from #12238 more robust since I saw a case where a diff changes only a single character in a chroma class instead of the entire thing. Add another more complicated test to match. Co-authored-by: Lauris BH <lauris@nix.lv>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
For diffs we first syntax highlight the before and after line then use a 3rd party diff library to find the difference in them and create a substring based on that, which we then highlight a 2nd time to show the specific difference within a line that has changed. In a specific case if the diffrence also changes the chroma class it will split in the middle of the attr and cause broken HTML:
Will then split on
Where the difference starts, and produce something broken like:
Fix that by detecting when it happens and putting the HTML back together properly before highlighting the added/deleted code.
Fixes #12235