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

Server-side syntax highlighting for all code #12047

Merged
merged 23 commits into from
Jun 30, 2020

Conversation

mrsdizzie
Copy link
Member

@mrsdizzie mrsdizzie commented Jun 24, 2020

This PR does a few things:

  • Remove all traces of highlight.js
  • Use chroma library to provide fast syntax highlighting directly on the server
  • Provide syntax highlighting for diffs + code comments
  • Re-style code view and unified/split diffs views to try and look a bit more 'modern'
  • Add custom syntax highlighting styling for regular theme and convert previous arc-green highlighting best I could

Fixes #7729
Fixes #10157
Fixes #11825
Fixes #7728
Fixes #3872
Fixes #3682

And perhaps gets closer to #9553

Suggestions welcome for styling

New Diff:

Screen Shot 2020-06-24 at 4 13 12 PM

Screen Shot 2020-06-24 at 7 31 29 PM

Screen Shot 2020-06-24 at 7 46 21 PM

Screen Shot 2020-06-24 at 7 43 10 PM

Screen Shot 2020-06-26 at 12 26 37 PM

This PR does a few things:

* Remove all traces of highlight.js
* Use chroma library to provide fast syntax hilighting directly on the server
* Provide syntax hilighting for diffs
* Re-style both unified and split diffs views
* Add custom syntax hilighting styling for both regular and arc-green

Fixes go-gitea#7729
Fixes go-gitea#10157
Fixes go-gitea#11825
Fixes go-gitea#7728
Fixes go-gitea#3872
Fixes go-gitea#3682

And perhaps gets closer to go-gitea#9553
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Jun 25, 2020
@lafriks lafriks added this to the 1.13.0 milestone Jun 25, 2020
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 25, 2020
@silverwind
Copy link
Member

I think you still need to run npm uninstall highlight.js and commit the results.

@mrsdizzie
Copy link
Member Author

I think you still need to run npm uninstall highlight.js and commit the results.

Thanks! fixed

@silverwind
Copy link
Member

Oh, and npm uninstall domino as well and remove this line.

@mrsdizzie
Copy link
Member Author

Oh, and npm uninstall domino as well and remove this line.

fixed

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 29, 2020
@mrsdizzie
Copy link
Member Author

@silverwind made a few tweaks to light theme based on the link you gave above -- think it is pretty good for now. Pygments doesn't have quite the level of detail that pretty lights does in some areas but is pretty close.

Also raised the threshold for highlighting to 1MB based on https://help.github.com/en/github/creating-cloning-and-archiving-repositories/limits-for-viewing-content-and-diffs-in-a-repository

@silverwind
Copy link
Member

Will likely need to undo to worker-loader removal (part of 901ce59) because #12095 will depend on it.

@mrsdizzie
Copy link
Member Author

Will likely need to undo to worker-loader removal (part of 901ce59) because #12095 will depend on it.

Do I need to do any special commands to properly add it back to npm, or just add the line back to package.json and run make frontend?

@silverwind
Copy link
Member

Run npm install worker-loader and add back the two removed sections in webpack.config.js and .eslintrc. npm install will take care of package-lock.json.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 30, 2020
@silverwind
Copy link
Member

Syntax looks good on light and dark. I have not encountered some of the more exotic identifiers in the files I was looking at but we should be good with those brighter color on arc-green.

@lafriks
Copy link
Member

lafriks commented Jun 30, 2020

🚀

@lafriks lafriks merged commit af7ffaa into go-gitea:master Jun 30, 2020
@mrsdizzie
Copy link
Member Author

Thanks all : )

mrsdizzie added a commit to mrsdizzie/gitea that referenced this pull request Jul 8, 2020
Fix regression casued by go-gitea#12047 so copy/paste works properly in all browsers.

Fixes go-gitea#12184

Also while looking at this I saw a small display issue for blame view. I think go-gitea#12023 was merged into original PR through an update branch before go-gitea#12047 was merged and made one of the css ruules not apply anymore.
zeripath pushed a commit that referenced this pull request Jul 8, 2020
* Make copy/paste work for source code

Fix regression casued by #12047 so copy/paste works properly in all browsers.

Fixes #12184

Also while looking at this I saw a small display issue for blame view. I think #12023 was merged into original PR through an update branch before #12047 was merged and made one of the css ruules not apply anymore.

* use pseudo-element to prevent copying of comment + symbol even when not visually selected

* remove added newline here should not be necessary anymore

* make sure empty line is newline so there is something to select and copy
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Server-side syntax hilighting for all code

This PR does a few things:

* Remove all traces of highlight.js
* Use chroma library to provide fast syntax hilighting directly on the server
* Provide syntax hilighting for diffs
* Re-style both unified and split diffs views
* Add custom syntax hilighting styling for both regular and arc-green

Fixes go-gitea#7729
Fixes go-gitea#10157
Fixes go-gitea#11825
Fixes go-gitea#7728
Fixes go-gitea#3872
Fixes go-gitea#3682

And perhaps gets closer to go-gitea#9553

* fix line marker

* fix repo search

* Fix single line select

* properly load settings

* npm uninstall highlight.js

* review suggestion

* code review

* forgot to call function

* fix test

* Apply suggestions from code review

suggestions from @silverwind thanks

Co-authored-by: silverwind <[email protected]>

* code review

* copy/paste error

* Use const for highlight size limit

* Update web_src/less/_repository.less

Co-authored-by: Lauris BH <[email protected]>

* update size limit to 1MB and other styling tweaks

* fix highlighting for certain diff sections

* fix test

* add worker back as suggested

Co-authored-by: silverwind <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
4 participants