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

typo fix #3500

Merged
merged 2 commits into from
Jan 11, 2019
Merged

typo fix #3500

merged 2 commits into from
Jan 11, 2019

Conversation

estelle
Copy link
Member

@estelle estelle commented Jan 10, 2019

<string> was considered a tag, so switched to <dfn><<string>></dfn> so it would be visible

<string> was considered a tag, so switched to <dfn><<string>></dfn>  so it would be visible
@dontcallmedom
Copy link
Member

Marked as non substantive for IPR from ash-nazg.

Copy link
Collaborator

@frivoal frivoal left a comment

Choose a reason for hiding this comment

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

The original was definitely wrong, good catch, and thanks for the patch.

As to the correction, I wonder what's better. What you did is one option, but we're not really defining the type <<string>> here, so maybe just marking it up as <<string>> (so that it links to css-values-3 would be more appropriate than <dfn><<string>></dfn>.

I know there's a handful of other places in other specs that follow the pattern use used, but it feels off to me, and I think they too should be corrected.

If we really want to enable cross-linking between the grammar and the definition, I think what would be more appropriate is to name this non terminal. Something like this:

<dt><dfn>path()</dfn> =
	path( [<<fill-rule>>,]? <<path-string>> )
</dt>
[...]
<dfn><<path-string>></dfn> is a <<string>> that represents an
<a href="https://www.w3.org/TR/SVG11/paths.html#PathData">SVG Path data string</a>.

I'm approving your PR nonetheless, because you're fixing a clear bug, while what I'm saying feels somewhat subjective. So feel free to merge if you disagree, and at least we'll get the bug fixed, but I think it'd be better either without a dfn, or with a named non-terminal.

@tabatkins
Copy link
Member

The correct markup for things like this is just to mark it as for the appropriate thing. That's used regularly in property definitions. I'll edit the PR and merge it.

And remove some extraneous end tags while I'm here.
@tabatkins tabatkins merged commit 0dea118 into w3c:master Jan 11, 2019
@estelle estelle deleted the patch-5 branch August 23, 2022 18:39
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.

4 participants