-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 HTML decoding #4146
More HTML decoding #4146
Conversation
source
Outdated
@@ -121425,10 +121425,11 @@ INSERT INTERFACES HERE | |||
Takashi Toyoshima, | |||
Takayoshi Kochi, | |||
Takeshi Yoshino, | |||
<span data-x="" lang="tr">Tantek Çelik</span>, | |||
<span lang="tr">Tantek Çelik</span>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove the data-x
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My beautifier identified it as empty when I cleaned that link. Is data-x
used for something specific when building the document?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's for cross-references, so in this case it's explicitly marking the span
as not being a cross-reference.
I think grouping them is fine, but some might need some discussion. E.g., −1 is hard to distinguish from -1 so I'm not sure if we should do that as it might lead to folks using -1 instead. (I've used |
What's the risk of someone using -1 rather than −1? Would it be better to use text like "negative 1" or "minus 1"? Happy to make the change. |
I'm mainly worried about introducing inconsistencies in formatting. I don't think switching to text helps as we also format equations with this character. Keeping the existing entity or switching it to |
Thanks, I think this can be merged after rebasing (feel free to force push after fixing). Not entirely clear why it conflicts. |
I think the conflict is the first commit which is already in master. |
@edent sorry we dropped this for a while. Do you think this is all the decoding work there is to do? If so, we can take care of the rebasing for you, since it's our fault for letting this sit. |
@domenic please can you rebase - I'm busy for the next week or so. Then I'll do some more decoding. |
Rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased again, and let's merge this :). Sorry again for the delay.
@edent it looks like your affiliation with the alphagov GitHub organization is not public; could you make it so per https://help.github.com/en/articles/publicizing-or-hiding-organization-membership to allow the IPR checker to sign off? |
@edent we'd need NHSX to sign the agreement, I think. Although maybe since the contribution was made while you were at alphagov and were their contact person we can override the bot in this case... |
Won't be able to get NHSX to sign any time soon, so if you could override the bot that'd be great. |
Re #3683
Would it be better if I did each of these commits as a separate PR? There will be quite a lot of them.
/acknowledgements.html ( diff )
/embedded-content-other.html ( diff )
/form-elements.html ( diff )
/images.html ( diff )
/text-level-semantics.html ( diff )