-
Notifications
You must be signed in to change notification settings - Fork 64
Implement highlight/strikeout/wrap for code listings #965
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
base: main
Are you sure you want to change the base?
Conversation
I'm also curious if you've considered adding the ability to configure line number display on/off with a custom setting like this? I feel like it would be super useful in combination with the new wrapping feature to more easily distinguish between soft wrapped line vs purposeful newlines in the actual code. |
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.
Hi @DebugSteven , amazing work! thank you so much for your PR!
I left some suggestions to refactor the code a bit but everything else is great!
As @mportiz08 mentioned, I also think that you should consider changing the code of the highlighting to match the tutorials ones :)
Excited for this PR!
@marinaaisa @mportiz08 I was the source for the different coloration - I wanted an option that was significantly more like a highlighter so that it would pull more attention, especially in a an article context where a reader is more likely to be skimming, so that it drew their attention to the relevant sections quickly. I totally get wanting to keep it consistent though, my biggest concern, especially with the dark mode viewing, is that the highlighting is too subtle - and only if you're really focused on the content will you capture that detail. We also wanted to extend beyond what the tutorial offers - for example, the tutorial content doesn't display a deleted line, which is sometimes a critical piece of what's happening in an explanation, hence reaching for the strikeout flavoring. |
9b73675
to
a74d9a1
Compare
I got some feedback on the forums that people would like to see a way to highlight errors. David showed some examples where people indicate errors using plain text comments. Would it be acceptable to add an additional option for error highlighting with that orange color? Would you like error highlighting to have the same opacity as the highlighting for lines and in tutorials? |
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.
The code looks pretty good to me so far right now—no obvious issues I'm noticing. I know that there is still some ongoing discussion on the syntax and JSON changes in the forums, but I think this PR is in good shape depending on how that discussion goes.
My only concern about the example screenshots is the one where content gets soft wrapped and the wrapped content starts at the very left edge of the container whereas I might expect it to start after the vertical space that is being used for the line numbers—pretty minor concern though and possibly could be addressed in the future (guessing it might be tricky to get it right).
Also, you may need to update the tests after merging in the latest main—we recently updated the testing library dependency which has some minor changes to the APIs for things like find
, etc
I don't have very strong opinions on this, but it looks like the highlight styling is already utilizing a CSS property for the color of the highlight, so hopefully this is something that can be easily extended for situations like that if we want to support it. |
I don't necessarily suggest that we implement error highlighting right now but I want us to think about how this feature would be able to grow to support those kinds of highlights. |
e786c79
to
56bbf95
Compare
@swift-ci please test |
ad09aa4
to
2ed9baa
Compare
@swift-ci please test |
…e listings * Implemented support for wrapping and highlighting individual lines * Added strikethrough (strikeout) annotations * Replaced specific line annotation styles with a unified `lineAnnotations` property * Updated default values and tests to reflect new line annotation behavior
2ed9baa
to
4f1cd4a
Compare
@swift-ci Please test |
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.
Awesome work! I think DocC users will love having these new capabilities :)
One minor visual thing I noticed is that the highlights can look a little less than ideal when the line numbers aren't enabled. (Looks great with line numbers enabled though!)

There isn't any spacing between the highlight edges and the content and the alignment between highlighted/non-highlighted lines is slightly off.
…he highlighted-border-width is present on all lines
@swift-ci please test |
} | ||
.code-listing:not(:has(.code-number)) .code-line-container { | ||
padding-left: $code-number-padding-left; |
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.
This seems to be adding spacing for normal, unnumbered code listings without any of the new styles applied.
I think you added this to account for the issue I found with aligning highlighted/non-highlighted lines. Is there a small tweak we can do to this so that this extra spacing is only added in that scenario?
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.
You're right, that is why I added it. I was having a difficult time figuring out a way to get the spacing and alignment looking right. I'm not sure if this was a reasonable way to approach it.
If it is though, could I add an additional :has(.highlighted)
, so it would be .code-listing:not(:has(.code-number)):has(.highlighted)
? That way the padding would apply to only code listings with highlighted lines in it.
Otherwise, do you have a different suggestion you'd prefer? I'm happy to investigate another approach.
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.
There might be a more efficient selector for this, but in general I think that's fine—as long as it correctly only applies the special padding when it is necessary. We can always try to improve the selector after once we get it working.
Thanks for looking into this—I feel like spacing in the code listings has always been especially tricky.
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.
Thank you. I've pushed up that change.
I verified the selector only applies when it should (highlighted lines, no line numbers, and with/out other options). Locally, I added a red outline for testing to make it obvious to me and confirmed it didn't appear for other cases (strikeout
/wrap
) or when showLineNumbers
is on.
.code-listing:not(:has(.code-number)):has(.highlighted) .code-line-container {
padding-left: $code-number-padding-left;
outline: 2px solid red; // for local testing
}
Screenshot below uses:
```shell, highlight=[1,2,3]
docc convert MyNewPackage.docc \
--fallback-display-name MyNewPackage \
--fallback-bundle-identifier com.example.MyNewPackage \
--fallback-bundle-version 1 \
--additional-symbol-graph-dir .build \
--output-dir MyNewPackage.doccarchive
```

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.
Awesome, I think we're super close now!
Sorry for being so tedious and detailed about this, but I'm noticing that there's still a slight difference in the left-hand spacing for code listings in the following 2 scenarios:
- main
- this branch with no experimental flag (has slightly more spacing compared to main even with this feature turned off)
The issue I found with the highlighted/non-highlighted alignment looks good now though! I think there just might be an additional spacing other than the padding now that needs to be conditionalized—possibly a transparent border that might also have been meant for numbered code listings or something.
@swift-ci please test |
Summary
This PR implements new code block options for line highlighting, line striking, line wrapping, and showing line numbers. When the
enable-experimental-code-block-annotations
flag is enabled, code listings will have access tohighlight
,strikeout
, andwrap
properties forwarded in the render JSON.User Experience
highlight
appear with the same highlight seen in tutorials.strikeout
appear with a strikethrough line.wrap
is present and greater than 0, code lines are soft-wrapped near the specified width.showLineNumbers
option is used on a code block, line numbers will render.Implementation Overview
highlight: [Int]?
,strikeout: [Int]?
,wrap: Int?
, andshowLineNumbers
(boolean) fromRenderBlockContent.codeListing
Dependencies
This PR depends on associated changes in
swift-docc
to actually render and handle thehighlight
,strikeout
,wrap
, andshowLineNumbers
options. The associatedswift-docc
PR is here: swiftlang/swift-docc#1287Testing
Setup
Use the additional-metadata branches for
swift-docc
andswift-docc-render
with the newhighlight
,strikeout
,wrap
, andshowLineNumbers
changes.Rebuild documentation using
swift-docc
and the feature flagenable-experimental-code-block-annotations
. Serve it using a local build ofswift-docc-render
.How to Test
highlight
, `strikeout`, `wrap` and/or `showLineNumbers` options after the language (optionally) like this:Checklist
npm test
, and it succeededThese new options looks like the below:


Compared to without these options:
