Skip to content
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

Highlight deleted code #515

Open
peterfriese opened this issue Mar 13, 2023 · 9 comments · May be fixed by #589 or swiftlang/swift-docc-render#664
Open

Highlight deleted code #515

peterfriese opened this issue Mar 13, 2023 · 9 comments · May be fixed by #589 or swiftlang/swift-docc-render#664
Labels
good first issue Good for newcomers new feature New features or new functionality

Comments

@peterfriese
Copy link

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;

peterfriese/MakeItSo#94 (comment)

Importance

No response

Alternatives Considered

No response

@peterfriese peterfriese added the new feature New features or new functionality label Mar 13, 2023
@mportiz08
Copy link
Contributor

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.

@mportiz08 mportiz08 transferred this issue from swiftlang/swift-docc-render Mar 23, 2023
@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented May 16, 2023

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.

image image
{
  "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

@Kyle-Ye Kyle-Ye added the good first issue Good for newcomers label May 16, 2023
@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented May 16, 2023

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 swiftlang/swift-docc-render#662.

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented May 16, 2023

A rough and incomplete implementation can be found at here #589 and swiftlang/swift-docc-render#664.

I'm not good at UX design. Maybe we can post a forum post seeking for help.

image

Some potiential UX design(Git-like code list) :

image

@natikgadzhi
Copy link
Contributor

@Kyle-Ye, you still working on this one, or want some help? I'd be interested in tinkering with it.

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Jun 4, 2023

Yeah, feed free to continue working on this.

@natikgadzhi
Copy link
Contributor

natikgadzhi commented Jun 4, 2023

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:

  1. 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.
  2. 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:

It feels that @Kyle-Ye's pull requests are really close to completion. I would like to do some cleanup, if it makes sense:

  • 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 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.
  • Comments in LineHighlighter. It has a diagram explaining it's logic, but otherwise private functions comments are sparse, happy to clean them up and elaborate.
  • Should we add documentation for highlights on swift.org after we get this merged?

@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.

@d-ronnqvist
Copy link
Contributor

I think the biggest next step here would be to start a thread in the Swift Forums to discuss:

  • how the diff should appear on the page (for example, should it be a side-by-side diff or inline diff?).
  • how the insertions and deletions should be reflected the updated Render JSON specification.

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 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.

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 optional reset parameter and set it to true.

@natikgadzhi
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers new feature New features or new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants