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

fix errors in docxtotei noticed by Stylesheets group #634

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Conversation

sydb
Copy link
Member

@sydb sydb commented Oct 10, 2023

While discussing #610 the Stylesheets group noticed two minor bugs, which this PR is intended to address.

  1. Read version from file VERSION (rather than a static hard-coded value that has not been changed since 2.15.0!).
  2. Fix typo — function had "#teitodocx" where it meant "#docxtotei" (alright, that is more than a typo, but you get the idea).

2) Fix typo — function had "#teitodocx" where it meant "#docxtotei"
@sydb sydb added type: bug A bug report. resp: StylesheetsGroup Issue is suitable for the group-learning approach taken in the Stylesheets Coop Working Group. labels Oct 10, 2023
@sydb sydb added this to the Release 7.56.0 milestone Oct 10, 2023
@sydb sydb requested a review from HelenaSabel October 10, 2023 21:15
@@ -20,7 +20,7 @@
</fileDesc>
<encodingDesc>
<appInfo>
<application xml:id="docxtotei" ident="TEI_fromDOCX" version="2.15.0">
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid such non-updated version statements, wouldn't it be nice to automatically update them on build or on release and, e.g., just fill in a placeholder here?

Copy link
Member

Choose a reason for hiding this comment

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

@bwbohl I think we've addressed this: We've added a rule to read the current version number in the Stylesheets in docxtotei.xsl.

<xsl:template name="generateAppInfo">
<appInfo>
<xsl:variable name="version"
select="if ( unparsed-text-available('../../VERSION') )
Copy link
Member

Choose a reason for hiding this comment

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

@bwbohl Here's where we made the change!

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks good!
Nevertheless, what I was getting at is manually maintaining TEI files in Test/expected-results/ vs. having a placeholder there for the value of @verison and then, when a text is executed, updating it on the fly from the VERSION-file; in the sense of "preparing" or "building" the test prior to execution. Doing so would guarantee that the expected-results @version is always in sync with the current VERSION and avoid the test from failing just due to the value of @version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, @bwbohl. That’s what I thought you might have meant. I think you are absolutely right. But I do not think we plan to do anything to improve comparison to files in Test/expected-results/, because I think we are going to stop using Test/ (in favor of using what is now called Test2/). There we will probably need to change the cleanForDiff target to remove that @version.

@HelenaSabel HelenaSabel merged commit a3b252c into dev Oct 26, 2023
4 checks passed
@raffazizzi raffazizzi added this to the Release 7.56.0 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resp: StylesheetsGroup Issue is suitable for the group-learning approach taken in the Stylesheets Coop Working Group. type: bug A bug report.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants