-
Notifications
You must be signed in to change notification settings - Fork 855
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
Code highlighting feature #304
Conversation
fixed lint warning for forgotten semicolon in gulpfile
add highlightJs gulp plugin add copying of styles for highlighting add highlighting to gulp prepare task
one performance tweak for gulp task
.gitignore
Outdated
@@ -1,3 +1,4 @@ | |||
.idea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s not related to project’s code, otherwise it might fill with VS Code, Sublime, Atom, Notepad++, etc. configs. There’s global .gitignore for that.
Suggestion: do not apply HTML highlight to every |
@pepelsbey at first, I have selector for code for highlighting '.slide pre:has(code)', not even 'pre' tag would be highlighted. |
@pepelsbey looks like all points regarding our talk are implemented. |
@pepelsbey any updates? |
Couldn’t find time to properly test it. Will try to have a look this week. Sorry. |
Right, I’m back. We have a few problems here, but nothing too scary :)
|
…eature/code-highlighter # Conflicts: # gulpfile.js
@pepelsbey looks like all was done |
@@ -0,0 +1,100 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not useful empty line
package.json
Outdated
"gulp-replace": "^1.0.0", | ||
"gulp-zip": "^4.0.0", | ||
"gulp-zip": "^4.1.0", | ||
"gulp-util": "^3.0.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use of the deprecated package https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
whole gulp community is trying to remove it from projects :)
package.json
Outdated
"merge-stream": "^1.0.0", | ||
"path-exists-cli": "^1.0.0" | ||
"path-exists-cli": "^1.0.0", | ||
"run-sequence": "^2.2.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a new package?
I do not see its use in PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s not used anymore since moving to Gulp 4.
gulpfile.js
Outdated
`$1${STYLES_DEST_PATH}$3`, { skipBinary: true } | ||
)) | ||
.pipe(highlightCodeSnippets()) | ||
.pipe(gulp.dest('')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error: Invalid dest() folder argument. Please specify a non-empty string or a function.
It gives an error, I guess it should be '.'
gulpfile.js
Outdated
/(<link rel="stylesheet" href=")(node_modules\/highlight\.js\/styles)\/(.*\.css">)/g, | ||
`$1${STYLES_DEST_PATH}$3`, { skipBinary: true } | ||
)) | ||
.pipe(highlightCodeSnippets()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s highlightCode()
now.
package.json
Outdated
"merge-stream": "^1.0.0", | ||
"path-exists-cli": "^1.0.0" | ||
"path-exists-cli": "^1.0.0", | ||
"run-sequence": "^2.2.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s not used anymore since moving to Gulp 4.
You suggest in docs to run |
fixed code issues fixed potential security issue with browser-sync using 'npm audit fix'
…eature/code-highlighter
@pepelsbey btw I removed info about how to run code highlight separately from docs because it included to preparate task by default. |
@nikolay-govorov, it seems code highlighting is another feature worth moving to CLI :) |
@nikolay-govorov what do think about such feature for shower-cli? |
I think it would be cool to have this in the CLI, but I'm not sure I know how to integrate it. Is it going to be something like the |
I’d just have it as |
But what will she do? Now archive and publish are building the presentation, not looking in the |
Hmm, then you’re right. It makes sense to use it as a flag, rather
|
We decided that such a feature does not align with the project’s vision. Presentation’s HTML is a source file that should be easy to write and understand. A generator that modifies the source file would hurt source code readability. We’d rather go manual (mark) or dynamically generated (highlight.js) code highlight path. I’m sorry it took the whole year to get back to this PR and decide. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
As @RubenVerborgh articulated in this comment, I don't see any reason for having syntax hightlight as either plugin or |
Topics to discuss:
<pre><code></code></pre>
snipeets without any class? (class needed to highlight as specific language, default is HTML)