Skip to content

Don't regenerate the html if the moFile changes. - #30

Closed
derekriemer wants to merge 2 commits into
nvdaaddons:masterfrom
derekriemer:master
Closed

derekriemer wants to merge 2 commits into
nvdaaddons:masterfrom
derekriemer:master

Conversation

@derekriemer

Copy link
Copy Markdown
Member

The mo/po process is parallell to the generation of the markdown/html documentation process. Documentation in the form of doc/[lang]/ is not dependent on mo/po files in lc_messages because the contents of the translated docs don't derive from pot/po files.

The mo/po process is parallell to the generation of the markdown/html documentation process. documentation in the form of doc/[lang]/ is not dependent on mo/po files in lc_messages  because the contents of the translated docs don't derive from pot/po files.
@josephsl

Copy link
Copy Markdown
Contributor

@XLTechie, any thoughts?

Comment thread sconstruct Resolved
@josephsl

josephsl commented Jun 17, 2023 via email

Copy link
Copy Markdown
Contributor

Comment thread sconstruct Outdated

langDirs = [f for f in env.Glob(os.path.join("addon", "locale", "*"))]

moFiles = []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@derekriemer did you test this PR against an add-on which does not have any existing translated files? Because I believe I tried this change when building PR #29, and something down the line (the dependency function?) couldn't handle an empty list.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that accidentally got left in from a previous hack. This pr removes moFile from being a dependency of html generation altogether. As it turns out, moFile is assigned inside a for loop iterating over a collection of directories. I don't see why it makes any sense to have html generation depend on 1, and only 1 mo file, from some random language. The reason I made this change is because the mo files are not storing translations from documentation, and I believe this entire process is in error. The translated md files live in doc/[lang]/readme.md and thus it doesn't make sense to make them regenerate if unrelated po files change, as they only need regenerated if the md files change themselves. If we are going to do it this way, annd there's a good reason to do this, the correct solution is to maintaina hash map of language mapping to the object needed to generate that language, and then look up the appropriate mo file for that language to include as a dependency. It doesn't make sense to make every html file depend on a mo file for the last language we saw in lc_messages/lang] as [lang] has nothing to do with all the other html files.

@XLTechie

Copy link
Copy Markdown
Member

@derekriemer It seems a lot of what you did in this was format changing/linting. That makes it hard to notice what you've actually changed.
So apologies if I have missed something, but it appears that your core change, is to un-do what was done in #28 and #29. Can you confirm that?

If so, can you please comment on the assertions in the comment by @abdel792 that was the reason behind #28, that the titles in HTML files do depend on the MO files? If that is the case, why shouldn't the HTML files be updated when the MO files are updated?

This process is a bit opaque to me, therefore if I have misunderstood something, which is highly possible, I am open to correction.

@derekriemer

Copy link
Copy Markdown
Member Author

I was not aware of #28 and thus this pr will be reverted. I'll submit a pr that does the correct thing, correctly makes each doc file depend on that languages moFile if such a moFile exists for that language.

@XLTechie

XLTechie commented Jun 17, 2023 via email

Copy link
Copy Markdown
Member

nvdaes pushed a commit to nvdaes/AddonTemplate that referenced this pull request Jun 9, 2026
* Update link to the GetText build for Windows

* Apply suggestion from @seanbudd

---------

Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@nvdaes nvdaes mentioned this pull request Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants