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

Update keyword linking script to handle P603 paragraphs #452

Merged
merged 4 commits into from
Jan 7, 2025

Conversation

hakonhagland
Copy link
Collaborator

These paragraphs contain verbatim monospaced text and should not be linked. See #446 (comment) for more information

@gdfldm
Copy link
Collaborator

gdfldm commented Jan 3, 2025

What if LibreOffice changes all of the style numbers?

@blattms
Copy link
Member

blattms commented Jan 3, 2025

Maybe in addition detect keywords in a path (between slashes or between slash and dot)?

These paragraphs contain verbatim monospaced text and should not be
linked.
@hakonhagland
Copy link
Collaborator Author

hakonhagland commented Jan 3, 2025

What if LibreOffice changes all of the style numbers?

@gdfldm Good point. The style P603 is defined like this:

<style:style style:name="P603" style:family="paragraph" style:parent-style-name="Text_20_body">
  <loext:graphic-properties draw:fill="none" draw:fill-gradient-name="gradient" draw:fill-hatch-name="hatch"/>
  <style:paragraph-properties fo:margin-left="0cm" fo:margin-right="0.318cm" fo:margin-top="0cm" fo:margin-bottom="0.051cm" style:contextual-spacing="false" fo:text-align="start" style:justify-single-word="false" fo:orphans="0" fo:widows="0" fo:hyphenation-ladder-count="no-limit" fo:text-indent="0cm" style:auto-text-indent="false" fo:background-color="transparent" style:shadow="none" style:writing-mode="lr-tb"/>
  <style:text-properties style:font-name="Liberation Mono" fo:font-size="8pt" style:font-size-asian="8pt" style:font-size-complex="8pt" fo:hyphenate="false" fo:hyphenation-remain-char-count="2" fo:hyphenation-push-char-count="2" loext:hyphenation-no-caps="false" loext:hyphenation-no-last-word="false" loext:hyphenation-word-char-count="5" loext:hyphenation-zone="no-limit"/>
 </style:style>

The script could search through all automatic-styles and determine if there is one or more styles that has for example:

  • style:family="paragraph"
  • style:parent-style-name="Text_20_body"
  • loext:graphic-properties sub element
  • style:text-properties sub element with
    • style:font-name="Liberation Mono"
    • fo:font-size="8pt"

and record the value of style:name (here P603) for such styles, or alternatively we could create a new custom style as we did with the NotKeyword style, and put that in the office:styles section (and not in office:automatic-styles as it is now)

Any other ideas for how to solve this?

Use some logic to determine if a style represents a mono font size paragraph
that should not be linked
@hakonhagland
Copy link
Collaborator Author

and record the value of style:name (here P603) for such styles

I chose to try out this approach first, see latest commit.

@hakonhagland
Copy link
Collaborator Author

Maybe in addition detect keywords in a path (between slashes or between slash and dot)?

@blattms Yes this is a good idea, but I suggest we delay the feature unless we have an explicit case that shows up where it is needed.

and self.style_paragraph_properties
and self.style_text_properties
and self.libre_mono_font
and self.libre_mono_font_size)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@hakonhagland: That then means the font size needs to be set to exactly 8pt such that the MonoParagraphStyle is valid, right? How about other font sizes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, the font size might be changed in the future. I suggest we allow any font size to be included in the MonoParagraphStyle

@lisajulia
Copy link
Collaborator

I generally approve this PR 👍 :), yet I have one remaining question: #452 (comment), as soon as that is answered, I'm happy to merge this.

Don't check for a fixed font size equal to 8pt as it might
change in the future.
@hakonhagland
Copy link
Collaborator Author

@lisajulia Thanks for the review. I have a addressed the comment in the latest commit

@lisajulia
Copy link
Collaborator

@lisajulia Thanks for the review. I have a addressed the comment in the latest commit

Thanks :) yet I'm wondering if the check for the font size could be removed overall, since we check if style:font-name is Liberation Mono? Sorry for the nitpicking...

@hakonhagland
Copy link
Collaborator Author

@lisajulia Removed the font size check.

@lisajulia lisajulia merged commit e4cd596 into OPM:main Jan 7, 2025
6 checks passed
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