Open peterfriese opened 1 year ago
Thanks for capturing this feature request @peterfriese! You're correct that the renderer currently only supports highlighting the added lines. I'm going to transfer this issue to the swift-docc repository since I believe that we would need compiler/JSON changes in order to properly support highlighting deleted lines.
For anyone interested, this should be implemented here https://github.com/apple/swift-docc/blob/main/Sources/SwiftDocC/Model/Rendering/Tutorial/LineHighlighter.swift
I also noticed some potential bug. If anyone is happy working on this, you may need to pay extra attention to the correctness of the current highlight behaviour.
{
"content": [...],
"fileName": "CustomizedSlothView.swift",
"fileType": "swift",
"highlights": [
{
"line": 25
},
{
"line": 26
},
{
"line": 27
},
{
"line": 28
},
{
"line": 29
}
],
"identifier": "01-creating-code-02-08.swift",
"syntax": "swift",
"type": "file"
}
The tutorial resource can be downloaded from https://developer.apple.com/documentation/xcode/slothcreator_building_docc_documentation_in_xcode
I also noticed some potential bug. If anyone is happy working on this, you may need to pay extra attention to the correctness of the current highlight behaviour.
Confirm this is a bug on swift-docc-render instead of swift-docc. I've filed up a bug via https://github.com/apple/swift-docc-render/issues/662.
A rough and incomplete implementation can be found at here #589 and https://github.com/apple/swift-docc-render/pull/664.
I'm not good at UX design. Maybe we can post a forum post seeking for help.
Some potiential UX design(Git-like code list) :
@Kyle-Ye, you still working on this one, or want some help? I'd be interested in tinkering with it.
Yeah, feed free to continue working on this.
Spent a bit of time digging deeper, here's what I see. Some of these will be obvious to everyone, just documenting my first steps.
Here's how things work now:
LineHighligher
in swift-docc
automatically diffs previous and next steps of tutorials or code blocks in them, and collects all the insertions
into highlights
property.swift-docc-render
picks up highlights
array, and applies the right css class to each line of code, depending on whether it's highlighted, in ContentNode/CodeListing.vue
.Here's what @Kyle-Ye has in progress:
deleteHighlights
that collects lines that were deletions
in the diff.It feels that @Kyle-Ye's pull requests are really close to completion. I would like to do some cleanup, if it makes sense:
highlights
, and all inserted lines would be highlighted, that's it. If we highlight both additions and deletions, should we rename highlights
, and use insertions
and deletions
on FileReference
? I have not looked at other places where these fields are used, and haven't checked if that will blow the scope on the PRs dramatically. LineHighlighter
. It has a diagram explaining it's logic, but otherwise private functions comments are sparse, happy to clean them up and elaborate.@mportiz08, does this feel like a good direction to you?
@Kyle-Ye, how does that sound? You did a lot of work on this, I want to make sure that I'm not blowing things up too much. If that feels right, I'll work on both swift-docc and the renderer side.
I think the biggest next step here would be to start a thread in the Swift Forums to discuss:
It doesn't have to be much more than a couple of sentences that introduce the addition (highlighting deleted lines in tutorial code diffs) and present the two topics that you want to discuss in that thread (on page appearance and Render JSON specification updates).
If you want, you can include suggestions/recommendations for what you feel that the UX should be and how this should be represented in the Render JSON spec. Otherwise you could phrase it more open ended as topics to be discussed without suggesting any particular answers yourself.
If you send me a "personal message" in the Swift Forums we can iterate on the initial Forums post there.
- Naming. Previously, we just had
highlights
, and all inserted lines would be highlighted, that's it. If we highlight both additions and deletions, should we renamehighlights
, and useinsertions
anddeletions
onFileReference
? I have not looked at other places where these fields are used, and haven't checked if that will blow the scope on the PRs dramatically.
There are two parts to these names; the properties in the source code and the properties in the RenderNode specification. For both we need to consider the backwards compatibility. For the source code I think we should add insertions
and deletions
and mark highlights
as deprecated and make it a computed property that returns the insertions only. That way, we preserve the previous API and its behavior for backwards compatibility.
Unfortunately we can't do the same for the RenderNode specification. What we want to do about backwards compatibility in the RenderNode spec is something that we should discuss in the Forums.
- Comments in
LineHighlighter
. It has a diagram explaining its logic, but otherwise private functions comments are sparse, happy to clean them up and elaborate.
To me, the additional information that would be most helpful here would be additional context about how the authored tutorials steps relate to the highlights are how the previous file is used and where it comes from.
That said, it might be difficult to know how to describe this without experimenting with tutorials and steps with code files a bit and stepping through the code to see how how the line highlighter is used and where the information comes from.
- Should we add documentation for highlights on swift.org after we get this merged?
I think the main place where this code highlights is documented today is the "Showing Differences Between Steps" section on the @Code
directive documentation page.
As far as I understand, deletions would be highlighted automatically without any additional effort or specific configuration from the documentation author. If so, my feeling is that the phrase "[...] and highlights the differences [...]" in the existing documentation is general enough that it covers both insertions and deletions from a documentation author's perspective.
DocC automatically compares the code for each step against the code of the previous step, and highlights the differences on the tutorial page so the reader knows what to change in their own code. This automatic comparison doesn’t happen on the first step of a section. You can force a comparison or override it for any step’s code by using the
previousFile
parameter to denote a specific file for DocC to use for comparison. If you don’t want to show any differences, provide the optionalreset
parameter and set it totrue
.
@d-ronnqvist, thank you for looking into this! Agreed that the path forward is to source input on the forums. Thank you for pointing out the backwards compatibility piece.
I'll write something up and tag you!
That said, it might be difficult to know how to describe this without experimenting with tutorials and steps with code files a bit and stepping through the code to see how how the line highlighter is used and where the information comes from.
I have an experiment with a few tutorials in mind — I hope to get some first-hand experience there, and document what spots that I stumbled on.
Feature Name
Highlight deleted code
Description
Highlight deleted lines (potentially using red background colour).
Motivation
Sometimes (especially when starting out from a template), you need to tell the reader to delete one of more lines of code. It seems like the renderer is not capable of highlighting deleted lines.
Here is an example;
https://github.com/peterfriese/MakeItSo/pull/94#discussion_r1134366287
Importance
No response
Alternatives Considered
No response