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

More extensions #60

Merged
merged 31 commits into from Apr 20, 2022
Merged

More extensions #60

merged 31 commits into from Apr 20, 2022

Conversation

2bndy5
Copy link
Collaborator

@2bndy5 2bndy5 commented Apr 11, 2022

  • added port of pymdownx.keys and pymdownx.tasklist to the theme.
    • sphinx_immaterial.keys is optional since proper usage requires pymdownx installed (for aliasing with pymdownx.keymap_db module) which is outlined in the new addition to the docs. closes Support for task lists #47
    • sphinx_immaterial.task_lists is auto enabled since it requires no external dependencies.
  • added a theme specific extension called theme_result in which a directive (.. rst-result::) is fed an RST snippet, and it is output as a literal code-block (with RST syntax highlighting) and the expected rendered output.
    • While this could be useful to other project's docs, the extension relies on some of the CSS provided by mkdocs-material theme. Some CSS was added to conform docutils output with the mkdocs-material's output.
    • Captions for code-blocks are patched to conform with mkdocs-material's code-blocks' captions (dubbed filename in the CSS classes).
    • docs have been modified to make use of the new rst-result directive where applicable.
  • Revised the CSS solution for wrapping multilined API signatures.
  • partial fix for Resolve roles in navigation #56 concerning content tabs' label accepting RST roles

@2bndy5 2bndy5 marked this pull request as ready for review April 12, 2022 00:36
.gitattributes Outdated
@@ -0,0 +1,8 @@
# Set the default behavior, in case people don't have core.autocrlf set.
* text=auto
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never use Windows, but why do we want any conversion of line endings at all? It seems like it would be be better to just have them always be LF at all times.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit was copied from the github docs. I can remove it, but it matches the settings I use globally (which I think is the default).

I added this file since npm run check spits out hundreds of errors from stylelint about the wrong line ending when I git checkout with windows.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you have core.autocrlf enabled?

I think we can disable any line ending conversion with:

* -text

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems risky without that safety net. It is a major pain in the ass to revert CRLF to LF manually if any get introduced (git diffs don't really show the distinction of different line endings - so I've been bogged down by this in the past).

I read yesterday (on the git docs) that the value input would convert CRLF to LF ("but not the other way around"). The docs are terribly worded to the point where the config option's explanation is actually confusing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not too hard to convert line endings if you find a problem --- e.g. you can use the dos2unix program.

I think the way you currently have it, autocrlf behavior will apply to all files by default if the user has core.autocrlf enabled, except that .py, .rst, and .scss files will always have LF line endings when checked out (even on Windows).

I'm not sure why we want some files to have autocrlf mode and some files always LF.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, I did some experimenting.

  • Setting my global config core.autocrlf=input provides LF endings in the local workspace on checkout. But this overrides the eol config option.
  • When eol is set to lf, then the workspace uses LF endings (which is the desired result - even on Windows), but this means autocrlf should be set to false (which means no normalizaton of line endings on checkout).
  • If core.autocrlf=false and core.eol=lf are set via .gitattributes, they seem to have no affect when the git global config is not core.autocrlf=false.

So, I think I found a compromise to override a user's global config for this repo:

  1. * text autocrlf=false prevents line ending normalization
  2. * text eol=lf ensures workspace uses LF on checkout

This way if a CRLF is introduced, then npm run check will likely fail. At that point the user should be aware of the line endings' preference for this repo and convert them manually as needed.

git docs links

This problem seems to have been addressed so many times (in various ways) that the docs are very nuanced and damn confusing.

docs/_static/extra_css.css Outdated Show resolved Hide resolved
sphinx_immaterial/kbd_keys.py Outdated Show resolved Hide resolved
sphinx_immaterial/kbd_keys.py Outdated Show resolved Hide resolved
@@ -45,7 +45,7 @@ $objinfo-icon-size: 16px;
font-weight: 700;
}

a.reference > .sig-name {
a.reference > span {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is okay but can you explain more why this is needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its related to the differences in domain. I found that reference links in C++ domain did not get special coloring (the intention of this CSS rule) when the highlight class (which is needed for other syntax highlighting like class) is added to the signature.

@jbms
Copy link
Owner

jbms commented Apr 18, 2022

Note: Going forward, it would probably simplify things if you try to keep unrelated changes in separate pull requests. Sometimes that isn't easy to do, but e.g. this PR could have been split into a number of separate ones:

  • rst-example directive
  • cpp domain fixes
  • task_list support
  • keys support
  • content tabs bug fix

@2bndy5
Copy link
Collaborator Author

2bndy5 commented Apr 18, 2022

Busted! I'm lazy about branch management in this regard. I have been trying to be better about this, but clearly it is a bad habit.

@jbms
Copy link
Owner

jbms commented Apr 18, 2022

I will often create multiple changes in the same working tree, and then split them into a sequence of commits. I often do this by staging individual files or individual hunks/line ranges. This works well as long as the changes don't affect nearby lines of the same file.

I use the Emacs magit package to stage individual hunks, but if using vscode it looks like you can do that as described here:

https://stackoverflow.com/questions/34730585/how-can-i-commit-some-changes-to-a-file-but-not-others-in-vscode

If you already have a sequence of commits, and then have made some changes in your working tree that you want to incorporate into that commit sequence, you can do that by staging the changes applicable to one of the commits, creating a fixup commit (e.g. git commit --fixup=commithash), not sure if there is a convenient way to do that with vscode, you can also just create a new commit, then use interactive rebase to apply it as a "fixup" to the existing commit.

@jbms
Copy link
Owner

jbms commented Apr 19, 2022

This all looks good to me, thanks! I just had one comment about the .sig-name CSS change.

@jbms jbms merged commit d874316 into main Apr 20, 2022
@2bndy5 2bndy5 deleted the more-extensions branch April 20, 2022 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for task lists
2 participants