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

Using ignore_list to ignore script and kbd #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

barrysteyn
Copy link
Contributor

There is no need to have specialized logic to ignore the script and kbd tags in the caps filter, it can be handled by the process_ignores function.

I have therefore added both script and kbd as defaults to be ignored in the process_ignores function, and removed the logic from the caps filter. Not only does this simplify the logic of caps, but on a higher level, it is a good idea to make sure that typogrify does not ever touch things like the script.

I have also added more tests, and moved some tests around. I am pleased to say that everything passes.

@chrisdrackett This is a great first step in simplifying the code, but while all the tests pass, you need to convince yourself that this is the way to go. I am available for discussions. The next step (which I will do if you are okay with this) is to use the smartypants tokenizer logic to skip html tags. This will simplify regex and logic of things like the amp filter.

@justinmayer
Copy link
Collaborator

@chrisdrackett: Any objections to using an actual HTML parser instead of regex? Seems like a good idea to me. As we all know, HTML can't be parsed via regex. ;^)

To wit, I'd rather have improvements to Typogrify occur in Typogrify itself, as opposed to vendoring our own typographical enhancements module in Pelican.

@barrysteyn
Copy link
Contributor Author

@justinmayer and @chrisdrackett
Looking at my source code, changing the Typogrify source code would require one to rewrite it from the ground up. Can I suggest that maybe we use what I have written as a starting point. I have also cleaned up the regex quite a bit, so it is much easier to read and understand, and far less complex.

@chrisdrackett
Copy link
Collaborator

@justinmayer none at all, in fact it would probably help with development (as I'm not a fan of regex at all).

@barrysteyn
Copy link
Contributor Author

@justinmayer and @chrisdrackett - please can you guys look at https://github.com/barrysteyn/pelican-typogrify
The readme will explain the main ideas and concepts. It is there to start a debate and (I hope) it will be used as a platform to continue this project.
I am away for a week starting this Sat, and then quite busy for the next three days after. Excuse me if I am quiet, but I promise I will be active after then.

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.

3 participants