OPM / opm-reference-manual

Other
1 stars 5 forks source link

Script to insert keyword links #410

Closed hakonhagland closed 1 week ago

hakonhagland commented 2 weeks ago

Builds on #408 and #409 which should be merged first.

The script replaces keyword names inside <text:p ...> tags with links to the corresponding keyword definition page. See #408 for more information. Since there are over 1000 keywords, and since I am not very confident that it will work correctly on every file, the plan is to apply the script to some keywords at a time. For example, starting with the 168 keywords in chapter 5. After it has been applied to all keyword files, we can consider applying it to the chapters and appendices also.

hakonhagland commented 2 weeks ago

There is currently an issue if the text contains fixed width fonts like for the H2STORE keyword. For example, at page 288 in the manual:

image

Do we want to insert links in the fixed width font part in the bottom of the screenshot? Currently, the script will insert links here too.

hakonhagland commented 2 weeks ago

The script also needs to handle recursive <text:p> tags. This is an issue if a paragraph contains footnotes that are also marked up with <text:p>.

gdfldm commented 2 weeks ago

Do we want to insert links in the fixed width font part in the bottom of the screenshot?

I would be inclined not to add links here. I think if we would like to have a link then the keyword could be mentioned in the text surrounding the example input.

gdfldm commented 2 weeks ago

Where a "keyword" appears multiple times in a KEYWORD.fodt file do we want add links to every occurrence or perhaps just the first?

hakonhagland commented 2 weeks ago

do we want add links to every occurrence or perhaps just the first?

@gdfldm We could also think of the links as a special markup for the keywords. Currently, the user recognizes a keyword by being an uppercase word, but there are upper case words that are not a keyword. However, if the keyword is also displayed in in e.g. italics and in a different color, the user will more easily recognize a keyword. Here is an example for how it will look for the H2STORE keyword:

image

Note that currently the keyword itself H2STORE is not marked up, but we could change that such that (it could link to itself) to avoid confusion (whether the H2STORE is a keyword or not).

blattms commented 2 weeks ago

Thanks a lot. I'll review this (with my limited Python knowledge) after the others are merged. That hopefully saves time.

hakonhagland commented 2 weeks ago

Currently, the script misses keywords that are split by span tags, for example if CO2STORE occurs like this within the text <text:span text:style-name="T6">C</text:span>O2STORE

hakonhagland commented 2 weeks ago

Rebased this onto #408

hakonhagland commented 2 weeks ago

Rebased this and added a commit that will generate the mapping from keywords to URIs on the fly, see discussion in https://github.com/OPM/opm-reference-manual/pull/409#discussion_r1829463086 for more information

hakonhagland commented 2 weeks ago

Added three more commits to address the review comments in #411 .

hakonhagland commented 2 weeks ago

Updated script to handle aliases, see https://github.com/OPM/opm-reference-manual/pull/411#issuecomment-2465607885 for more information

blattms commented 1 week ago

Seems like this needs a rebase, sorry.

Hopefully, the number changed files is smaller after this. 15 seems a bit high.

hakonhagland commented 1 week ago

Hopefully, the number changed files is smaller after this.

@blattms Actually this PR includes #417, so maybe we should merge that first? @gdfldm can you have a look at this: https://github.com/OPM/opm-reference-manual/pull/417#issuecomment-2468170738

Then, I can rebase this one after that one has been merged.

blattms commented 1 week ago

Ok the other PR is merged. Please rebase this and I hope that @lisajulia will take another look at the Python code.

hakonhagland commented 1 week ago

Ok the other PR is merged. Please rebase this

@blattms Thanks! I rebased this now.

hakonhagland commented 1 week ago

I suggest to name the files differently

@lisajulia Thanks for the review :) I have renamed the files in the last update.