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

Added glsxtrfull from glossaries-extra #234

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

JulianGoeltz
Copy link
Contributor

Uses parsing from glossaries file.
Commit includes test coverage.

@torik42 torik42 added the type: LaTeX Issues concerning supported or new LaTeX command or packages label Nov 7, 2022
@torik42
Copy link
Owner

torik42 commented Nov 10, 2022

If I understand correctly, the command adds the full form and abbreviation in parentheses independent of where it is called. Is that correct? Is there any reason, you didn’t add the abbreviation?

@JulianGoeltz
Copy link
Contributor Author

It seems I didn't fully grasp the functions in glossaries.py and just looked at the test. Your understanding of the glossaries command is correct, how would one best implement this for YaLafi?

@torik42
Copy link
Owner

torik42 commented Nov 15, 2022

Let me first explain what happens in glossaries.py. From your fork I assume you know how Macro handler functions work, otherwise read this first. The glossary entries are written to the_glossary (the way that works doesn’t matter here). For each command, the macro handler function f (line 112 in current master 852f7eb) is constructed using the two arguments key and mods in h_gls (line 111). key corresponds to the key in .glsdef. get_tokens (line 113) get the tokens of the values matching the key in .glsdef; in line 120 they get expanded; if mods=[], line 121 is irrelevant, o.w. the functions in mods (either cap_all or cap_first) are applied. Finally, the positions of the new tokens are set to the \ of the corresponding Macro, e.g. \gls. Later, such positions are automatically extended to the full command.

My suggestion would be to first define \glsxtrshort and glsxtrlong similar how you did it (could you import the necessary functions form glossaries with from glossaries import …). These seem to use the short and long keys. For \glsxtrfull one can then add \newcommand{\glsxtrfull}[1]{\glsxtrshort{#1} (\glsxtrlong{#1})} assuming this is how it works.

I haven’t properly tested these things and don’t know what exactly I should expect from using the commands, so please check again with running real LaTeX and then add tests with the correct behaviour to test_glossaries-extra.py.

@JulianGoeltz
Copy link
Contributor Author

Thanks for this insightful explanation and the suggestion, I think I implemented it, see the new commit. And sorry again for the initial PR, that was full of errors.
I crosschecked the current assumptions in the test with a latex file:

gls \gls{pp}


glsxtrshort \glsxtrshort{pp}

Glsxtrshort \Glsxtrshort{pp}


glsxtrlong \glsxtrlong{pp}

Glsxtrlong \Glsxtrlong{pp}


glsxtrfull \glsxtrfull{pp}

Glsxtrfull \Glsxtrfull{pp}

image
I adapted tests/test_packages/glossaries-extra.tex for this, should I commit this as well?

As an aside: the reason why I personally used \glsxtrfull is that it does not change the first use flag, i.e. when using \setabbreviationstyle[acronym]{long-short} a use of \gls after \glsxtrfull will still print the long and short version, and I used this in the figures in the paper to have the explanations of acronyms both in the text and captions.

@torik42
Copy link
Owner

torik42 commented Nov 16, 2022

Thank you for the contribution, this looks very good now! I would just like to squash them again before merging (or you do so, if you want, see e.g. here).

I adapted tests/test_packages/glossaries-extra.tex for this, should I commit this as well?

That’s not necessary for now, we also don’t have them in glossaries.tex. At some point, I might update test_glossaries_extra.py to the new .glsdef format. But for now, this is enough.

@torik42
Copy link
Owner

torik42 commented Nov 16, 2022

Just one thing. Could you add the new macros to list-of-macros.md?

@JulianGoeltz
Copy link
Contributor Author

Added to list-of-macros.md and squashed :)

@torik42
Copy link
Owner

torik42 commented Nov 16, 2022

Just wanted to merge and noticed one more thing: In your LaTeX output \Glsxtrfull{pp} produces ‘ppm (Parts per million)’, this seems stupid to me and might reveal a bug in glossaries-extra, but YaLafi produces ‘Ppm (…)’.

Could you change that and then also add commas in list-of-macros.md?

Implementation via glsxtrshort and glsxtrlong.
Commit includes test coverage.
@JulianGoeltz
Copy link
Contributor Author

Oh, sorry. I did look at this but than apparently missed it again. Thanks for spotting!
Added the commas as well

@torik42 torik42 merged commit b106af0 into torik42:master Nov 16, 2022
@torik42
Copy link
Owner

torik42 commented Nov 16, 2022

Thank you for the nice contribution! Successfully merged into master.

@JulianGoeltz JulianGoeltz deleted the feature_glsxtrfull branch November 16, 2022 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: LaTeX Issues concerning supported or new LaTeX command or packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants