remove trailing space in codeblock lines #204
Conversation
✅ Deploy Preview for neo4j-docs-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Can we also hook to the copy event so that people doing ctrl-c don't get a broken behavior?
src/js/06-code.js
Outdated
| @@ -46,6 +46,8 @@ document.addEventListener('DOMContentLoaded', function () { | |||
| input = window.neo4jDocs.copyableCommand(input) | |||
| } | |||
|
|
|||
| input = input.replace(/[ ]+\n/g, '\n').trimEnd() | |||
There was a problem hiding this comment.
| input = input.replace(/[ ]+\n/g, '\n').trimEnd() | |
| input = input.replace(/[\s]+?\n/g, '\n').trimEnd() |
Might be worth testing with this, in case somebody decided to use TAB to space annotations from code. A ? might be needed to ensure \s doesn't eat the carriage return as well.
There was a problem hiding this comment.
(Edited to provide more context)
I think it's worth noting here that we know of only one code block where the space between the code and the callout causes an issue. It might not be only curl that errors, but in most cases I think if you copy and paste some lines of code and there are whitespace characters at the end of lines you are not going to have a problem.
If someone has used tab to space annotations from code we should change that in the asciidoc. We could also resolve this by not having a space in the source before the callout, and adding spacing by updating the css.
Another possible fix is to have guidance to not use callouts in code blocks where the language is curl. (Generally this shouldn't be much of an issue, because these code blocks are unlikely to be very long or require a large number of callouts).
There was a problem hiding this comment.
May I add some comments here?
- I find this callout feature really nice to explain complex code or shell commands blocks!
- right, it is not just curl what would cause errors, but at least all shell commands where a line break character ("") is used before the callout
- The problem with shell commands and spaces at the end is the shell error message which doesn't help at all to understand the problem. And spaces are invisible 😄. This would lead to user frustration.
I would like to use callouts so hope this will be fixed - Thx!
There was a problem hiding this comment.
If it helps get this out sooner, I would say we just focus on the copy button behavior and the use of a single space as that's the issue that has been identified in this specific case. Further work can be done in a separate PR.
There was a problem hiding this comment.
I think [ |\t] will work for us here if we want to also catch tabs. @stefano-ottolenghi please let me know if that misses anything obvious.
I've also added a listener for the copy event when the button either isn't used or isn't available.
There was a problem hiding this comment.
Looks good to me! There's only a commented-out line we should remember to remove, but otherwise good work! 🤸
There was a problem hiding this comment.
Thanks - I've deleted the line now
No description provided.