Provide GfW ReleaseNote Start menu Icon, and tweak collapsible div marker

[PhilipOakley]

Improve the accessibility and readability of the release notes.

1. Add a start menu icon for the GfW's own release notes, so use can find and read them.

2. Small improvement to the readability of the notes by using larger, open 'white' triangles for the collapsible regions of the release notes - feels more like the typical explorer look. Previously they looked like basic bullet points

3. Also, specifically mark the upstream Git's link as being for their release notes (not tested successfully - may not have invoked the please.sh appropriately. I'd expected the sdk build installer to do it's magic - guidance needed!)

Signed-off-by: Philip Oakley philipoakley@iee.email

[dscho]

What was the problem with sdk build installer?

[PhilipOakley]

The sdk installer definitely built an installer. I was able to run it.
It installed the icon.
The release note that it linked to had the new larger open 'white' triangle.

It (ReleaseNotes.html) just didn't have the extra words for the parts where the upstream Git is linked (actually patch 2 - that was my mistake).

I think that I may need to do something else additional to get that part of the release notes formatted. I suspect that's somewhere in the please.sh. I'm just not sure what I need to do for that case.

git grep please.sh in /usr/src/build-extra gives me a list of potential candidates... maybe need to get to ./please.sh finalize release-notes. dunno.

If I get that working, the one extra I'd like to do (separate patch) would be to prefix each of the collapsible divs which start "Changes since.." with the "Git vX.Y.Z: " that is being considered. (My eyes cross over seeing the release before, so go to the wrong place..)

So at the moment I feel I have two good patches, one that I've failed to test, and one that's at just a plan.

[PhilipOakley]

I have removed the extra commit that was not working for me. I'll investigate that at some other time.

I also updated the PR title, once I found that edit via the web interface.

[dscho]

I think that I may need to do something else additional to get that part of the release notes formatted. I suspect that's somewhere in the please.sh. I'm just not sure what I need to do for that case.

Are you referring to https://github.com/git-for-windows/build-extra/blob/master/render-release-notes.sh?

[dscho]

Very nice! Thank you! Please let me know whether you want to act on my suggestion(s), or whether you think it's not necessary.

[dscho]

Maybe a space between "Release" and "Notes"?

(BTW this icon will open the release notes in the default web browser app, which is not a web app.)

[PhilipOakley]

I swithered about the spacing, and took the easy route of just using the filename 😉

You are right about needing 'browser' in their. I had a senior moment. previously I'd even written 'seb'.

I can fix both.

[dscho]

I just had another look, and once these two issues are addressed, I think this PR is good to merge.

[dscho]

It (ReleaseNotes.html) just didn't have the extra words for the parts where the upstream Git is linked (actually patch 2 - that was my mistake).

Oh, oh, I think I know what you mean. For new Git for Windows releases based on a new Git version, we add that link. And yes, that's done in please.sh, in the finalize release-notes function:

build-extra/please.sh

Lines 4075 to 4088 in 4ecc9eb

	case "$next_version" in
	*.windows.1)
	v=${next_version%.windows.1} &&
	if ! grep -q "^\\* Comes with \\[Git $v\\]" \
	"$sdk64"/usr/src/build-extra/ReleaseNotes.md
	then
	url=https://github.com/git/git/blob/$v &&
	txt="$(echo "${v#v}" | sed 's/-rc[0-9]*$//').txt" &&
	url=$url/Documentation/RelNotes/$txt &&
	mention feature 'Comes with [Git '$v']('$url').'
	fi ||
	die "Could not mention that Git was upgraded to $v\n"
	;;
	esac

[PhilipOakley]

Do you (I) have to run please.sh finalize release-notes manually to actually create a fresh version of the .html before running the installer?

I am thinking that there is process or script part that creates the html, and that it's the html that is included in the user-installer (rather than the .md file bing in the installer and the installer itself does some processing at install time).

When I looked at please.sh I found the various mention() calls, one of which was for the upstream Git, and I tried to (did) amend that, hoping that it would be able to use the existing .md file, and 'improve' the readability of the .html (for less knowledgable readers - the old & slow 😉). I think it was that extract you have quoted, but without fully appreciating the context. I think I had it right, but had failed to ensure the script part was run (as per my initial Q above).

[dscho]

Do you (I) have to run please.sh finalize release-notes manually to actually create a fresh version of the .html before running the installer?

Only if you want an entry "Comes with Git v[...]" in the latest item of the release notes. Did I misunderstand and you did not want that?

I am thinking that there is process or script part that creates the html, and that it's the html that is included in the user-installer (rather than the .md file bing in the installer and the installer itself does some processing at install time).

Yes, render-release-notes.sh translates the .md to .html. This is done as part of installer/release.sh. The .md is not included in the installer.

When I looked at please.sh I found the various mention() calls, one of which was for the upstream Git, and I tried to (did) amend that, hoping that it would be able to use the existing .md file, and 'improve' the readability of the .html (for less knowledgable readers - the old & slow 😉). I think it was that extract you have quoted, but without fully appreciating the context. I think I had it right, but had failed to ensure the script part was run (as per my initial Q above).

Indeed, I did not yet get a chance to look at your patches.

[PhilipOakley]

I've updated the two patches as discussed, and tested locally using $ sdk build installer, abd the resultant Installer is available as C:\Users\phili\Git-0-test-64-bit.exe installed just fine and both the initial release notes display, and doing the same via the new start menu icon, had the updated triangle icons and changing mouse over cursor.

So looking good for me.

PS I'll now see if I can get a start on the tweaks to the formatting to work. I'll extend my description so we can make sure we are on the same page talking about the same problem 😉

[PhilipOakley]

Do you (I) have to run please.sh finalize release-notes manually to actually create a fresh version of the .html before running the installer?

Only if you want an entry "Comes with Git v[...]" in the latest item of the release notes. Did I misunderstand and you did not want that?

I am thinking that there is process or script part that creates the html, and that it's the html that is included in the user-installer (rather than the .md file bing in the installer and the installer itself does some processing at install time).

Yes, render-release-notes.sh translates the .md to .html. This is done as part of installer/release.sh. The .md is not included in the installer.

When I looked at please.sh I found the various mention() calls, one of which was for the upstream Git, and I tried to (did) amend that, hoping that it would be able to use the existing .md file, and 'improve' the readability of the .html (for less knowledgable readers - the old & slow 😉). I think it was that extract you have quoted, but without fully appreciating the context. I think I had it right, but had failed to ensure the script part was run (as per my initial Q above).

I've realised I had completely the wrong mental model of what was happening...
I had thought that the .md file (the listings for all the previous versions) was in a terse format, and the please script expanded the terseness to a clean concise text, which was then rendered for html. That is not correct (I think..).

What I'd seen in the please script was actually a bit of code that adds the new entries for latest release to the head of the .md file. And that those new entries, like the older entries, was already in the 'concise' style (I'd been looking at the browser formatted HTML, which doesn't show the actual link address on the page, hence thinking it was 'terse').

So I think I know what I'm doing now. I'll an initial proof of concept pair that modify the please finalise that updates the mention Git line to active state that the implied link is to the release notes, and a patch that updates the added 'Changes since' header to add the start version to become 'Git v2.27.0: Changes since..' (this is because these headers are below the fold on the page, so you can't tell which version you are looking at, only the previous 'since' version;-).

Once those two work, I'll add two preparatory patches that goes through the older entries an tweak them to the new format (almost like a 'correct all spellings' patch). I'll do them on a new PR.

Thanks.

[dscho]

Once those two work, I'll add two preparatory patches that goes through the older entries an tweak them to the new format (almost like a 'correct all spellings' patch). I'll do them on a new PR.

Sounds good! In the meantime, I'll merge this PR.