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

Manual: add mark / highlight documentation #8194

Closed
wants to merge 2 commits into from

Conversation

wlupton
Copy link
Contributor

@wlupton wlupton commented Jul 22, 2022

Please see this "Syntax for <mark>?" pandoc-discuss thread for context.

  • I've added a new section after (and modeled on) the existing underline and small caps sections
  • I've checked that <span class="mark">Mark</span> does indeed work (and uses the mark element)
  • But is it true that this will work in all output formats that support mark / highlight? (I don't know... I'm just asking)

Also note:

  • The pandoc-discuss thread points out that the mark class only works when it's the first class (and I've verified that this is also the case for the underline and smallcaps classes). Is this intentional (in which case it should be documented) or is it a bug?

  • The default HTML template defines underline and smallcaps classes, but it doesn't define a mark class. This seems a bit inconsistent? My "try pandoc" below illustrates that in the "mark isn't the first class" case the HTML will use the (defined) underline and smallcaps classes and the (undefined) mark class

  • I used this "try pandoc" to check out the various behaviors

@jgm
Copy link
Owner

jgm commented Jul 23, 2022

Duplicate of #8191 , merged yesterday?
If you think that commit needs modification, maybe you could rebase your PR against it.

@wlupton wlupton force-pushed the feature/document-mark-class branch from 52e3b2b to 7baa698 Compare July 24, 2022 11:39
@wlupton
Copy link
Contributor Author

wlupton commented Jul 24, 2022

I think that the natural location for this new text is with the similar underline and smallcaps subsections at the end of the Inline formatting section.

  • I've replaced my original commit with a new one that moves the newly-added text and adjusts the last line to match the last lines of the underline and smallcaps sections
  • I've added a second commit that defines a span.mark HTML style

The following comment is still open:

  • The pandoc-discuss thread points out that the mark class only works when it's the first class (and I've verified that this is also the case for the underline and smallcaps classes). Is this intentional (in which case it should be documented) or is it a bug?

@wlupton
Copy link
Contributor Author

wlupton commented Jul 24, 2022

It looks like the test failures are due to the change to styles.html?

@jgm
Copy link
Owner

jgm commented Jul 25, 2022

Why add the span.mark style? The HTML writer will use a <mark> tag in this case. (Never mind: I see you addressed this above.) Is it to handle the case where mark is not the first class? That should probably be fixed in the writer instead. (Or maybe the behavior should be kept, but then I think it's confusing to add span.mark.)

jgm added a commit that referenced this pull request Jul 25, 2022
Previously classes like "underline" and "marked" had to
be the first class in a span in order for the span to be
interpreted as a "ul" or "mark" element.  This commit allows
these special classes to be "stacked," e.g.
`[test]{.mark .underline}`; in addition, the special classes are no
longer required to come first in the list of classes.

See #8194 for context.
@jgm
Copy link
Owner

jgm commented Jul 25, 2022

OK, I've pushed a commit that allows these special classes to be combined in a single span. They are no longer required to be first in the list of classes. I think this means we could remove the special CSS for e.g. span.underline, but I want to check with @mb21 first.

@wlupton
Copy link
Contributor Author

wlupton commented Jul 25, 2022

Re the span.mark style, maybe your "never mind" means that I don't need to say anything about it, but anyway:

  • I added it because underline, smallcaps and mark seem to be a triple that are handled similarly, and there were already span.underline and span.smallcaps styles

  • I had noted that, in the case where the special class wasn't first, then the span.underline class kicked in and resulted in the expected formatting, but the lack of a span.mark class meant that 'mark' spans didn't get special formatting

  • If the special class no longer needs to be first then I agree with your rationale for getting rid of the span.underline class (and not adding the span.mark class)

  • Note that the span.smallcaps class is still needed, because AFAIK the only way to get smallcaps is via CSS (there's no HTML element)

@mb21
Copy link
Collaborator

mb21 commented Jul 26, 2022

I think this means we could remove the special CSS for e.g. span.underline

You mean the span.underline selector ? yes, if we now always output <u> instead of <span class="underline"> doesn't seem to be needed anymore. Although I think that was changed quite some time ago in #6277 ...

@tarleb
Copy link
Collaborator

tarleb commented Jul 26, 2022

This is probably a good time to bring up the very related PR #7307 again.

@wlupton
Copy link
Contributor Author

wlupton commented Apr 10, 2023

I was going through my open items and found this. I'm inclined simply to close (withdraw) this PR. If you think that this would be a pity (and that we should continue and resolve the discussion on this issue) then please speak up!

@tarleb
Copy link
Collaborator

tarleb commented May 4, 2023

Thank you, this is indeed a better place for that section. Just to make sure: is the CSS still used after all the rebases? I'm probably missing something, as I didn't see why it's needed.

@wlupton
Copy link
Contributor Author

wlupton commented May 4, 2023

I see that the only span style in the latest templates/styles.html is span.smallcaps (so span.underline must have been removed). I therefore withdraw any suggestion that we should add a span.mark style.

@tarleb
Copy link
Collaborator

tarleb commented May 4, 2023

I've git cherry-picked the commit onto main as 9c27262.

Thanks again!

@tarleb tarleb closed this May 4, 2023
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