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

Diff in the web interface #13342

Closed
2 of 6 tasks
savatin opened this issue Oct 28, 2020 · 8 comments · Fixed by #13357
Closed
2 of 6 tasks

Diff in the web interface #13342

savatin opened this issue Oct 28, 2020 · 8 comments · Fixed by #13357
Labels

Comments

@savatin
Copy link

savatin commented Oct 28, 2020

  • Gitea version (or commit ref):
    1.14.0+dev-18-gd453533be
  • Git version:
    2.26.2
  • Operating system:
    Ubuntu 18.04.3 LTS
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
  • Log gist:

Description

The ' and " characters are displayed incorrectly when diff files are displayed in the web interface:
https://try.gitea.io/sergey.savatin/test_diff/commit/62d96b3486efecfce9736c7c3382077c10bfa6c4

Screenshots

image

@silverwind
Copy link
Member

HTML source looks rather broken too with a 0x03 character in there:

<span class="mono wrap">    sh �<span class="added-code">9;useradd -u $(stat -c "%u" .gitignore) jenkins'</span>;</span>

@zeripath
Copy link
Contributor

zeripath commented Oct 28, 2020

The issue is that the diff library appears to be diffing the html entities at the subentity level.

so &#34; is being compared with &#39; which means you get &#3 separated from the 9; one side and 4; on the other.

@zeripath
Copy link
Contributor

I think we should just be letting chroma do the highlighting by passing in a HighlightRanges() and adjusting the format that chroma.LineHighlight sets.

@zeripath
Copy link
Contributor

zeripath commented Oct 28, 2020

I'm too tired to do this right now but the idea would be rewrite gitdiff.go GetComputedInlineDiffFor to generate the line patches first and then pass to another highlight.Code - which would do the addition and deletion bits with the appropriate bits highlighted by calling the HighlightRanges and adjust LineHighlight options as appropriate.

@zeripath
Copy link
Contributor

(@mrsdizzie you don't have to fix - but if you understand what I'm getting at feel free to have a go.)

@mrsdizzie
Copy link
Member

Grr yea : ( I'm away for a few days but could take a look after

@zeripath
Copy link
Contributor

ugh the highlight trick suggested above doesn't work.

zeripath added a commit to zeripath/gitea that referenced this issue Oct 29, 2020
Fix go-gitea#13342

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor

OK I worked it - we just have to iterate through the diffs and run a fix up for the broken entities.

Now doing this I note that we end up creating these line diffs twice - which seems excessive to me - so clearly there are improvements to be made here.

@6543 6543 added the type/bug label Oct 31, 2020
techknowlogick added a commit that referenced this issue Oct 31, 2020
* When creating line diffs do not split within an html entity

Fix #13342

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Add test case

Signed-off-by: Andrew Thornton <art27@cantab.net>

* improve test

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
zeripath added a commit to zeripath/gitea that referenced this issue Oct 31, 2020
…#13357)

Backport go-gitea#13357

* When creating line diffs do not split within an html entity

Fix go-gitea#13342

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Add test case

Signed-off-by: Andrew Thornton <art27@cantab.net>

* improve test

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
lafriks pushed a commit that referenced this issue Oct 31, 2020
…13375)

Backport #13357

* When creating line diffs do not split within an html entity

Fix #13342

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Add test case

Signed-off-by: Andrew Thornton <art27@cantab.net>

* improve test

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@go-gitea go-gitea locked and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants