Restore "Edit on GitHub" button

[kousu]

Checklist

GitHub

• I've given this PR a concise, self-descriptive, and meaningful title
• I've linked relevant issues in the PR body
• I've applied the relevant labels to this PR
• I've applied a release milestone (major, minor, patch) in line with Semantic Versioning guidelines
• I've assigned a reviewer

PR contents

• I've consulted SCT's internal developer documentation to ensure my contribution is in line with any relevant design decisions
• I've added relevant tests for my contribution
• I've updated the relevant documentation for my changes, including argparse descriptions, docstrings, and ReadTheDocs tutorial pages

Description

Linked issues

Fixes #3490

[kousu]

https://github.com/logos

GITHUB®, the GITHUB® logo design, OCTOCAT® and the OCTOCAT® logo design are exclusive trademarks registered in the United States by GitHub, Inc.

The OCTOCAT design is the exclusive property of GitHub, Inc and has been federally registered with the United States Copyright Office. All rights reserved.

No adaptation or use of any kind of any of our registered trademarks or copyrights, or any other contents of this website, is allowed without the express written permission of GitHub, Inc.

For more information regarding the authorized uses of these items please contact us.

https://raw.githubusercontent.com/twbs/icons/main/icons/github.svg is

The MIT License (MIT)

Copyright (c) 2019-2021 The Bootstrap Authors

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

unclear how these interact.

also this icon is nice but it's 16x16 while the icons that come with furo are 24x24 -- though that /shouldn't/ matter since they're all vector graphics anyway.

[joshuacwnewton]

https://github.com/logos

Given what they have written on that page:

Do these awesome things

• Use the Octocat or GitHub logo to link to GitHub
• Use the Mark in social buttons to link to your GitHub profile or project
• Use the Octocat or GitHub logo to advertise that your product has built-in GitHub integration
• Use the Octocat or GitHub logo in a blog post or news article about GitHub

Obligatory IANAL, but I feel like the GitHub logo exists as a concept beyond the literal file github.svg, so I would think that GitHub's recommendations take precedent over the license for that one file. (i.e. I think we're fine to use the GitHub logo in this context.)

[kousu]

This is ready to go in. Here's the default view:

Here's dark mode in mobile view:

Clicking the icon takes you to the proper page to edit on GitHub:

I resurrected this part of our conf file

spinalcordtoolbox/documentation/source/conf.py

Lines 134 to 139 in d04d459

	html_context = {
	# TODO: when the Github icon is supported natively by furo (https://github.com/pradyunsg/furo/discussions/114)
	# then this should be moved into html_theme_options and the theme_ prefix should be dropped
	"theme_source_repository": "https://github.com/spinalcordtoolbox/spinalcordtoolbox",
	"theme_source_branch": "master", # or subprocess.check_output(["git", "rev-parse", "--abbrev-ref", "HEAD"]) ?
	"theme_source_directory": "documentation/source/",

but as mentioned there, those values should be up in html_theme_options -- I just wasn't able to get that that to work. They're not allowed there until furo declares knowing them as options.

[jcohenadad]

very cool @kousu ! thanks for the contribution

[joshuacwnewton]

Lovely work! ❤️

The bulk of the work LGTM, but I had a few questions/comments about the nitty-gritty details.

[joshuacwnewton]

Ahhh! Now I understand why you wanted to include the license for this! I thought we were just linking a image of the GitHub logo... I didn't realize the SVG image you were planning on adding was made by programmatically generating the shape of the GitHub logo.

(TIL about SVG in html... sorry about the misunderstanding earlier.)

[joshuacwnewton]

I looked over the changes you made and the core idea here seems fine!

Just to make sure I'm understand correctly, though:

• Internally the furo theme has its own _templates directory containing the original copies of all of these files, right?
• And, by having our own _templates directory, we're overriding the default templates (to include the "Edit on GitHub" changes you made)?

[joshuacwnewton]

One suggestion: To make these changes easier to understand in the future, maybe one thing we could do is add HTML comments before/after to make it apparent which blocks are ours, and which blocks come from Furo:

Suggested change

	<div class="edit-button-container">
	<a class="edit-button" href="{{ determine_page_edit_link() }}">
	<button type="button" class="btn-github" title="{{ _('Edit this page') }}">
	<svg><use href="#svg-github"></use></svg>
	</button>
	</a>
	</div>
	<!-- start of changes added by @kousu in https://github.com/spinalcordtoolbox/spinalcordtoolbox/pull/3624 -->
	<div class="edit-button-container">
	<a class="edit-button" href="{{ determine_page_edit_link() }}">
	<button type="button" class="btn-github" title="{{ _('Edit this page') }}">
	<svg><use href="#svg-github"></use></svg>
	</button>
	</a>
	</div>
	<!-- end of changes added by @kousu in https://github.com/spinalcordtoolbox/spinalcordtoolbox/pull/3624 -->

That way, a dev looking back on this work doesn't have to diff with the original Furo file to see what the changes were.

[kousu]

I started out by pasting in furo's page.html, then adding a commit that replaced the buttons with {{ include 'partials/buttons.html' }}, then a third commit that added in the patch I wanted to see. So, in that case, you could easily tell what had happened. But I was defeated having to redo the same for the mobile header icons -- they're not quite able to share the same partial because of how the CSS is set up. So I just backed everything out and did it again.

As an aside: in this case, sphinx-book-theme scores a few points over furo: it makes heavier use of partials and makes customizations a lot easier, and easier to track. I wish furo did too; I think it will eventually because furo's author is hard at work on componentizing all themes in https://github.com/pradyunsg/sphinx-basic-ng/tree/main/src/sphinx_basic_ng/theme/basic-ng/components it's just not been backported there yet.

I could redo the current version to have a 'import from furo' commit followed by a 'patch in the github button' commit. Would that be satisfying?

[joshuacwnewton]

I could redo the current version to have a 'import from furo' commit followed by a 'patch in the github button' commit. Would that be satisfying?

I think that would work! But, up to you, really. It's a bit of a nitpick... and it's possible that the next time we return to this will be to just remove it anyway (i.e. once it becomes native in furo).

[kousu]

Ah we have squashes forced on this repo so I couldn't do it that way afterall

[joshuacwnewton]

My comments have been more or less addressed! I think you can probably take it from here. :)