7904082: The UI for API Diff could be improved

[lahodaj]

This PR intends to improve the result of API diff in a few ways:

• in the list of updated elements, using green for elements that were added (instead of elements that are unchanged), and red for removed elements. At least in cases two variants of an API are compared.
• making added/removed elements more prominent by also coloring the signature lines
• hiding unchanged elements by default.

An example diff that is produced before this patch:
https://cr.openjdk.org/~jlahoda/CODETOOLS-7904082/before/
after this patch:
https://cr.openjdk.org/~jlahoda/CODETOOLS-7904082/after/

Please let me know what you think. I'll see if I can reasonably add some tests for this functionality.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• CODETOOLS-7904082: The UI for API Diff could be improved (Enhancement - P4)

Reviewers

• Christian Stein (@sormuras - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/apidiff.git pull/32/head:pull/32
$ git checkout pull/32

Update a local copy of the PR:
$ git checkout pull/32
$ git pull https://git.openjdk.org/apidiff.git pull/32/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 32

View PR using the GUI difftool:
$ git pr show -t 32

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/apidiff/pull/32.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lahodaj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

7904082: The UI for API Diff could be improved

Reviewed-by: cstein

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[mlbridge]

Webrevs

• 04: Full - Incremental (0b50d978)
• 03: Full - Incremental (ffd79499)
• 02: Full - Incremental (6c795e1e)
• 01: Full - Incremental (fcfba786)
• 00: Full (15e9e4a3)

[irisclark]

Thanks for working on apidiff!

Can you also provide a an example of the "after" for the case when unchanged elements are included? (That's what I'd need to use for the Platform JSR Annex 2 (e.g. https://cr.openjdk.org/~iris/se/25/latestSpec/apidiffs/index.html).). I think if you're changing the default, there's probably a corresponding change you'll need to make in src/share/doc/apidiff.md, which is also published here: https://openjdk.org/projects/code-tools/apidiff/apidiff.html.

In the list of updated elements, the background for the "!=" is now appears purple-y grey (formerly pinkish) and the "!=" character itself is red (unchanged, I think). I think this combination isn't as easy to parse as the former, Perhaps it needs more context to appreciate?

The change in SourceCodeAnalysis.SnippetWrapper which adds the background color for the notes "Only in baseline; not in: demo" and "Only in: demo; not in: demo" looks good. (I noticed that there's a typo in report/html/resources/report.properties, line 126 "not i" should be "not in".)

I'm not sure I'm finding an example of coloring of the signature lines. Let me know which elements I should be looking at for the comparison.

Thanks again for all of your work to improve apidiff!

[lahodaj]

Thanks for working on apidiff!

Thanks for looking at the PR!

Can you also provide a an example of the "after" for the case when unchanged elements are included? (That's what I'd need to use for the Platform JSR Annex 2 (e.g. https://cr.openjdk.org/~iris/se/25/latestSpec/apidiffs/index.html).). I think if you're changing the default, there's probably a corresponding change you'll need to make in src/share/doc/apidiff.md, which is also published here: https://openjdk.org/projects/code-tools/apidiff/apidiff.html.

The diffs produced after this PR hide the unchanged elements using JavaScript - i.e. the unchanged elements are still in the page, just hidden. And there's a checkbox in the upper right corner to show them "Show unchanged". The state of the checkbox is not persisted when one clicks on links, which is not very nice, but my inclination was that it would be rare for people to look at stuff that didn't change.

Not sure if that's acceptable for the platform annex, if not I can add a command line option to either always show the unchanged elements, or to show them by default.

(The current value of the "Show unchanged" checkbox is unchecked, so JavaScript will by default hide the unchanged elements. But, as the hiding is done by JavaScript, if JavaScript is disabled, the unchanged elements are shown, which I think might be helpful, as without JavaScript, we can't control the state.)

In the list of updated elements, the background for the "!=" is now appears purple-y grey (formerly pinkish) and the "!=" character itself is red (unchanged, I think). I think this combination isn't as easy to parse as the former, Perhaps it needs more context to appreciate?

I tried this:

• the background colors are green for added, red for removed, light gray for changed, and white for unchanged.
• the foreground color is black

(And I made the green and red slightly darker.)

Not sure if that looks good, I've changed the original diff with these colors:
https://cr.openjdk.org/~jlahoda/CODETOOLS-7904082/after.01/
and also produced a JDK25 to master diff:
https://cr.openjdk.org/~jlahoda/CODETOOLS-7904082/25-master/after.01/
and the JDK 25 to master before this PR:
https://cr.openjdk.org/~jlahoda/CODETOOLS-7904082/25-master/before/

Does this look better?

The change in SourceCodeAnalysis.SnippetWrapper which adds the background color for the notes "Only in baseline; not in: demo" and "Only in: demo; not in: demo" looks good. (I noticed that there's a typo in report/html/resources/report.properties, line 126 "not i" should be "not in".)

(I've fixed the typo, but the examples above were generated before fixing that, sorry. Thanks for finding that!)

I'm not sure I'm finding an example of coloring of the signature lines. Let me know which elements I should be looking at for the comparison.

What I meant was the colors in the "Only in ..." lines, sorry for being imprecise. I can try to extend the color to other lines as well in desired.

Thanks again for all of your work to improve apidiff!

[lahodaj]

I've:

• added a new command line option, --show-unchanged to unconditionally show unchanged elements
• added a test attempt

[lahodaj]

/integrate

[openjdk]

Going to push as commit cb6c45e.

[openjdk]

@lahodaj Pushed as commit cb6c45e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.