Skip to content

Conversation

@mjsir911
Copy link

This lets users properly highlight man(1) references even if guesswork doesn't report it as a manref. The offending links looked something like this:

@@RXVT_NAME@@perl(3)

Being able to wrap these in L<> to bold the entire string is a nicety.

A (too) thorough writeup, exploration, & justification of this PR can be found here

TODO:

  • fix tests
  • add another test outlining specific functionality
  • find a better way to apply bold than manual format codes?

This lets users properly highlight man(1) references even if guesswork
doesn't report it as a manref. The offending links looked something like
this:

@@RXVT_NAME@@Perl(3)

Being able to wrap these in `L<>` to bold the entire string is a nicety
}
} elsif ($$attrs{type} eq 'man') {
my ($page, $section) = $text =~ /^([^(]+)(?:[(](\d+)[)])?$/; # Copied from Simple.pm
return '\f(BS' . $page . '\f(BE\|' . "($section)";
Copy link
Author

Choose a reason for hiding this comment

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

Is there a better way to apply the bold formatting than using the format codes?

@mjsir911 mjsir911 changed the title Unconditionally style contents of L<> tags Unconditionally style contents of L<> tags Apr 25, 2023
@rra rra closed this in ae0820b Oct 24, 2023
@rra
Copy link
Owner

rra commented Oct 24, 2023

Thank you for the analysis and the initial patch! I have a committed a fix for this that will be in the next release.

The problem was a bit more complex than your patch, because by the time cmd_l gets the text, it has already been formatted, including man page references. To ensure the formatting is correct, cmd_l needs to use the underlying link attributes and reproduce the formatting. Otherwise, it risks double-formatting the reference in some edge cases or, more likely, missing some formatting. I think I found a way to do this that should work reliably, and added a test for it.

@mjsir911
Copy link
Author

mjsir911 commented Nov 3, 2023

Thank you @rra!

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.

2 participants