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

Render reports with styled notes containing subscript and strikethrough #1762

Open
wants to merge 2 commits into
base: maintenance/gramps52
Choose a base branch
from

Conversation

hgohel
Copy link
Member

@hgohel hgohel commented Sep 3, 2024

Fixes #13417

With this change all reports should be able to render styled notes using subscript or strikethrough without throwing exceptions.

  1. Add support for subscript and strikethough in DocBackend.
  2. Implement support for subscript and strikethrough in:
  • CairoBackend
  • LatexBackend
    Use package ulem for strikethrough support. ulem overrides rendering of emphasis, so specified normalel option to maintain the default.
    A bug in str_inc was being triggered during my testing. I used AI to fix the code and it seems to work. I haven't investigated whether this will change anything, but the code seems to work. Looking for review on the code change, ways to test, or other comments
    Update: AI generated code reverted as it is explicitly forbidden by Gramps contribution rules. Bug #13418 filed to track this issue separately as it is not the target of the bug being addressed by PR.
  • HTMLBackend
  • ODFBackend
    Add two new styles GSub and GStrikethrough to support subscript and strikethrough formatting, and implemented the two in odfdoc.py

Testing

  1. Create a note that includes subscript and strikethrough, and attach to a person
  2. Run any report which would include that note. Example: Complete Individual Report for that person
  3. Render report in all formats and verify that (1) no exceptions are thrown and (2) output is rendered appropriately for the format. For Text and RTF, these styles will not have any effect, other formats including PDF, HTML, LaTeX, ODT, PS you will see the style applied.

Todo

  1. Add methods similar to start[end]_superscript for subscript and strikethrough in each of the Doc classes so that plugins and addons can use the same functionality. For compatibility, this should be introduced in 5.3 so plugins can specify Gramps version required.
  2. Add similar API for italics since no API is available for that today. See thread on discourse: https://gramps.discourse.group/t/reports-normal-italic-and-bold-within-a-paragraph/5106/5

@@ -200,7 +204,6 @@ def escape(self, preformatted=False):
def find_tag_by_stag(self, s_tag):
"""
:param s_tag: object: assumed styledtexttag
:param s_tagvalue: None/int/str: value associated with the tag
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no s_tagvalue parameter to the method, so I removed this comment.

@hgohel
Copy link
Member Author

hgohel commented Sep 3, 2024

@emyoulation once the fix is merged, we ought to add subscript and strikethrough markup to a note in the Gramps example database. Thanks.

@hgohel hgohel added the bug label Sep 3, 2024
@emyoulation
Copy link
Contributor

I've had a pedigree 'breadcrumb_list' item on the back-burner for quite a long time now. (Partly because the way Perplexity suggested for superscripts was an awkward substitution key method and not terribly cross-compatible.)

Will this give a more flexible option?

Is it viable to add SmallCaps styling too? That's the other common Genealogy report styling to distinquish surnames from other name parts.

It would give Gramps a full complement of typical differentiating stylings for surnames: bold, underline, italic, uppercase. Small caps would be like the other stylings... it would disappear when pasted into a plain text editor. Upper case has to be hamd-converted.

See
https://gramps.discourse.group/t/commonly-seen-genealogical-reports-that-are-not-supported/3367/11

@hgohel
Copy link
Member Author

hgohel commented Sep 3, 2024

Will this give a more flexible option?

Is it viable to add SmallCaps styling too?

Yes, I believe so and it would be best handled as a new feature request.

This PR is a bug fix for an exception being thrown while generating reports with subscript & strikethrough being used in a Note. Previously @Nick-Hall implemented support for these styles in the Note editor with #1266 which worked well, and to fully integrate across gramps features, DocBackend needed to support the stylings as well.

Smallcaps styling could follow the same pattern - implement support in the note editor, DocBackend and its implementations, and add the APIs (which remains a todo item for Gramps 5.3) to enable plugin and addon authors to use the styling as well.

@wroldwiedbwe
Copy link

I used AI to fix the code and it seems to work.

FYI I believe that is against the projects Howto: Contribute to Gramps

Certify that you personally authored the code and did not copy from other implementations or use AI generators

@PQYPLZXHGF
Copy link
Contributor

PQYPLZXHGF commented Sep 3, 2024

FYI I believe that is against the projects Howto: Contribute to Gramps

Yes, it is against the policy and a large number of open source projects are struggling with this and in most cases any person who attempts to contribute ai generated code is essentially attempting to taint the projects code base and in a number of cases those people are blocked from contributing 😢 which is not what you really want in an open source project.

Even in this pr both the submitter and commenter are using ai's; the submitter does not mention which but the commenter has used at least [Perplexity.ai] see Gramps discussion (“Lumpers” and “Splitters” in Gramps, 2 drafts from Perplexity.ai) which in this touches on the user manual! So it is difficult to say what each of the ai's copyright material they trained on!

Please note this is not a personal attack on either of the people involved here and more of comment on ai and contribution that probably needs to be discussed and sorted out, like maybe making an small ai model that only trains on Gramps code?

@hgohel
Copy link
Member Author

hgohel commented Sep 3, 2024

@wroldwiedbwe @PQYPLZXHGF and @Nick-Hall
The code in this PR is all handwritten by me, with the exception of the changes to the method str_inc which was fixed by AI, as explicitly stated up front in my notes. I will remove the changes to that method and update this PR.

The result will be that the existing defect str_inc will continue to throw an exception if triggered, but the changes I made to support subscript and strikethrough are independent and complete.

@Nick-Hall
Copy link
Member

Yes. Please remove the code that was written by the AI.

We introduced this policy in April 2023 to avoid any potential future legal issues.

As copyright law evolves to clarify issues relating to AI and code development, we may wish to revisit this discussion.

@hgohel
Copy link
Member Author

hgohel commented Sep 4, 2024

Reviewers: with commit 51c1b3b the AI generated code has been removed. The defect that leaves in generating LaTeX reports is being tracked with bug #13418

Please use squash and merge when committing this PR and if possible, remove references to AI in the notes before committing. Thanks.

@wroldwiedbwe
Copy link

wroldwiedbwe commented Sep 6, 2024

Follow up discussion on Gramps forum

@emyoulation
Copy link
Contributor

Yes, I believe so and it would be best handled as a new feature request.

Thanks. Filed feature request.
0013430: Note format Add styling support for Small Caps

Fixes #13417

With this change all reports should be able to render styled notes using subscript or strikethrough without throwing exceptions.

1. Add support for subscript and strikethough in DocBackend.
2. Implement support for subscript and strikethrough in:
   a. CairoBackend
   b. LatexBackend
      i. Use package ulem for strikethrough support. ulem overrides rendering of emphasis, so specify normalel option to maintain the default.
     ii. A bug in str_inc was being triggered during my testing. I used AI to fix the code and it seems to work. I haven't investigated whether this will change anything, but the code seems to work. Looking for review on the code change, ways to test, or other comments
   c. HTMLBackend
   d. ODFBackend
      i. Add two new styles GSub and GStrikethrough to support subscript and strikethrough formatting, and implemented the two in odfdoc.py

Testing
1. Create a note that includes subscript and strikethrough, and attach to a person
2. Run any report which would include that note. Example: Complete Individual Report for that person
3. Render report in all formats and verify that (1) no exceptions are thrown and (2) output is rendered appropriately for the format. For Text and RTF, these styles will not have any effect, other formats including PDF, HTML, LaTeX, ODT, PS you will see the style applied.

Todo
1. Add methods similar to start[end]_superscript for subscript and strikethrough in each of the Doc classes so that plugins and addons can use the same functionality. For compatibility, this should be introduced in 5.3 so plugins can specify the version required.
2. Add similar API for italics (emphasize) since no API is available for that today. See thread on discourse: https://gramps.discourse.group/t/reports-normal-italic-and-bold-within-a-paragraph/5106/5
Code fix to str_incr() which was generated by AI has been reverted as this is explicitly disallowed by Gramps contribution rules.
@hgohel hgohel force-pushed the bug-13417-subscript-strikethough-support branch from 51c1b3b to c2afea9 Compare October 16, 2024 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants