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

Enable syntax highlighting on diff view #2528

Merged
merged 2 commits into from
Feb 7, 2016
Merged

Enable syntax highlighting on diff view #2528

merged 2 commits into from
Feb 7, 2016

Conversation

andreynering
Copy link
Contributor

Closes #733

@@ -830,6 +830,10 @@ $(document).ready(function () {

// Highlight JS
if (typeof hljs != 'undefined') {
// creating aliases for languages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this is needed?

Highlight.js tries to detect the language automatically. But in this particular case, it didn't work as expected. So, I'm explicity setting the language to Highlight.js, as a language-{{language-extension}} class. Unfortunally, not all languages are being reconized by its file extension (see). So I manually created aliases to two cases I found, but for most languages it isn't necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know that it was already implemented. ¯_(ツ)_/¯ Updated to use that package, much better.

I also did this patch to Highlight.js, so futures version will have these alises by default: https://github.com/isagalaev/highlight.js/pull/1069/files

There was just a little gotcha: I had to move highlight.go to its own package, because we got a cycle import, which isn't allowed in Go, on importing template in models since models also import template.

@unknwon
Copy link
Member

unknwon commented Jan 31, 2016

Any screenshots?

@unknwon unknwon added the status: needs feedback Tell me more about it label Jan 31, 2016
@unknwon unknwon added this to the 0.9.0 milestone Jan 31, 2016
@andreynering
Copy link
Contributor Author

Any screenshots?

Sure.

Before:
wo1
After:
w1

Before:
wo2
After:
w2

@unknwon
Copy link
Member

unknwon commented Feb 1, 2016

Takes a bit more time to review :trollface:

@unknwon unknwon removed the status: needs feedback Tell me more about it label Feb 1, 2016
@unknwon unknwon changed the title Enable sintax highlighting on diff view. Enable syntax highlighting on diff view Feb 1, 2016
@@ -7,9 +7,9 @@ github.com style (c) Vasily Polovnyov <vast@whiteants.net>
.hljs {
display: block;
overflow-x: auto;
padding: 0.5em;
/*padding: 0.5em;*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think overwrite this value in _base.less is better as we can't care very well when upgrading the plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unknwon Makes sense. Done!

@unknwon unknwon added the status: needs feedback Tell me more about it label Feb 3, 2016
@unknwon unknwon removed the status: needs feedback Tell me more about it label Feb 7, 2016
@unknwon
Copy link
Member

unknwon commented Feb 7, 2016

Thanks!

unknwon added a commit that referenced this pull request Feb 7, 2016
@unknwon unknwon merged commit f15a2f9 into gogs:develop Feb 7, 2016
@unknwon
Copy link
Member

unknwon commented Feb 7, 2016

@andreynering andreynering deleted the diff-sintax-highlight-733 branch February 7, 2016 17:01
@unknwon unknwon mentioned this pull request Feb 7, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants