Fix extra empty line issue on tutorial #663
Conversation
|
@swift-ci please test |
| @@ -192,7 +192,7 @@ function duplicateMultilineNode(element) { | |||
|
|
|||
| // wrap each new line with the current element's class | |||
| const result = getLines(element.innerHTML) | |||
| .reduce((all, lineText) => `${all}<span class="${className}">${lineText || '\n\n'}</span>\n`, ''); | |||
| .reduce((all, lineText) => `${all}<span class="${className}">${lineText || ''}</span>\n`, ''); | |||
There was a problem hiding this comment.
Hm. This change likely fixes one bug but it unfortunately re-introduces another that we previously had. These newlines were added to fix a bug where empty lines in a multiline Swift string were accidentally getting stripped.
With this code listing as an example:
let multiline = """
Needs
Spaces
Between
Lines
"""On main, this gets rendered how it looks there.
On this branch, this gets rendered as follows:
let multiline = """
Needs
Spaces
Between
Lines
"""Ideally we could find a solution that fixes #662 without re-introducing this other bug, although I know it's tricky because of the complex logic we have to handle line numbers and syntax highlighting.
There was a problem hiding this comment.
I think #502 was the PR for the bug fix that this change might accidentally re-introduce for context.
There was a problem hiding this comment.
No. This PR does not re-introduces the bug you mention. It passed the existing test and I tested this form and confirm it locally.
Because the original use nothing. #502 add
|| "\n\n". But actually add|| ""is enough.
There was a problem hiding this comment.
Unfortunately it does re-introduce the issue:
In your docs, of you have this:
let multiline = """
Needs
Spaces
Between
Lines
"""It will generate this:
There was a problem hiding this comment.
Unfortunately it does re-introduce the issue:
In your docs, of you have this:
let multiline = """ Needs Spaces Between Lines """It will generate this:
Got it. I thought it occur on the tutorial side.😂 And it was OK in tutorial steps.
There was a problem hiding this comment.
We should probably re-visit the way we render line numbers and the css styling around that.
We have a display:flex with a column wrap on the <code> element, which is probably not a good appraoch, since we are messing up the white space handling entirely, hence why I had to add that double /n/n while looping over the multi-lines.
There are two good ways to do this from my research:
- The traditional table rendering each line approach
- Move line numbers to a list after the code and leave some space for them.
Each has it's PROs and CONs.
|
Close. Prefer to use #667 |
Bug/issue #, if applicable:
Close #662
Summary
See the full context on #622
Another way to fix the problem is changing the logic here
src/components/ContentNode/CodeListing.vueA later fix to implement a clipboard button may help lift the issue.
#449
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
npm test, and it succeeded