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

Consecutive capitals in Math are labeled as span caps #3512

Closed
bryanwweber opened this issue Feb 17, 2021 · 3 comments · Fixed by #3516
Closed

Consecutive capitals in Math are labeled as span caps #3512

bryanwweber opened this issue Feb 17, 2021 · 3 comments · Fixed by #3516
Assignees
Labels
Milestone

Comments

@bryanwweber
Copy link
Contributor

Environment

Python Version: 3.9.1

Nikola Version: 8.1.3

Operating System: macOS

Description:

In Nikola 8.1.2, the caps typogrify filter was re-enabled. Unfortunately, consecutive capital letters in math mode (when enabled) also trigger the filter, resulting in broken equations. I think the simplest fix is to add the .math CSS class to the ignored tags here:

ignore_tags = ["title"]

Aside from that, in trying to debug this issue, I could not determine how to pass options to the typogrify_custom filter. typogrify_custom is the only filter wrapped by apply_to_text_file that has more than one argument, and it is not wrapped by the _ConfigurableFilter class to read an option from conf.py. In the end, I had to create a functools.partial in my conf.py that passed the list of typogrify_filters to apply as well as the modified ignore_tags.

If that's the intended interface, it would be nice to support a default list of applied filters, in case the user only wants to edit the ignore_tags (and vice-versa).

Please let me know if I should create a pull request with my suggested fix for the .math issue!

References:

Cantera/cantera-website#128
#3447 (and linked issues)

bryanwweber added a commit to bryanwweber/cantera-website that referenced this issue Feb 17, 2021
@Kwpolska
Copy link
Member

Please let me know if I should create a pull request with my suggested fix for the .math issue!

Sure, go ahead!

(This kinda depends on the project and the maintainers, but I personally prefer and recommend just creating minor/bugfix PRs without asking if they’re appropriate — worst case scenario, they’d get closed, but they are likely to just get accepted; note that policies differ in different projects; projects that mandate having a issue ticket first exist too.)

In the end, I had to create a functools.partial in my conf.py that passed the list of typogrify_filters to apply as well as the modified ignore_tags.

This might be a bit complicated, but I don’t think many users would be interested in using typogrify_custom — note that it requires importing callables from typogrify, so it’s not that easy to configure, functools.partial aside. If you have any ideas on how this could be improved, preferably without adding configuration options, we can discuss them (here or in a PR with your implementation).

@bryanwweber
Copy link
Contributor Author

bryanwweber commented Feb 18, 2021

@Kwpolska Thanks for replying! I opened a PR for the bugfix. merging! As you said, I'm never sure about the culture of various projects 😄 Apologies for the extra noise!

In any case, we can continue discussing a possible UI for typogrify_custom here. I don't mind using functools.partial in the conf.py, as you say, not many people will probably need to use this feature, and if it was documented it wouldn't be a problem. I think the main thing is to allow specifying only one argument or the other. Right now, the list of filters is required, so you do have to import everything from typogrify and specify it.

One way to approach this is to give a default None value to typogrify_filters and fill in the default list if it's not overridden.

@apply_to_text_file
def typogrify_custom(data, typogrify_filters=None, ignore_tags=None):
    """Run typogrify with a custom list of fliter functions."""
    if typo is None:
        req_missing(['typogrify'], 'use the typogrify filter', optional=True)
        return data
    if typogrify_filters is None:
        typogrify_filters = [typo.amp, typo.widont, typo.smartypants, typo.caps, typo.initial_quotes]
    return _run_typogrify(data, typogrify_filters, ignore_tags)

Then, in conf.py:

from nikola import filters
import functools
FILTERS = {".html": [functools.partial(filters.typogrify_custom, ignore_tags=["tag-to-ignore"])]}

The only other question I would have is should the user-passed list of ignore_tags be appended to the default ["title", ".math"], or replace it? On the one hand, the user may not expect that things that worked before are broken now. On the other hand, it is inconvenient if you want those defaults to remain to have to copy them out of the source code. Maybe the solution is just to document the defaults.

@Kwpolska
Copy link
Member

I’m okay with allowing None for the filter list. It would also be great to document functools.partial and implement appending to ignore_tags — could you open a PR for all three things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants