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

Make textcomp package register with textmacros automatically. #1073

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Mar 9, 2024

This PR causes the textcomp package to add itself to the textmacros packages automatically. In order to allow it to work with \require{textcomp} (and to work better with the Lab), we do this through the config() function of the configuration rather than setting it via options. This is because when \require{textcomp} is processed, its dependecies are loaded and added to the configuration first, then textcomp is added. Because textmacros is a dependency, it will be added and configured before textcomp adds to the textmacros.packages if it is placed in the options.textmacros.packages configuration option. Here, using config(), we know that textmacros will already be configured, so we can add to the textConf configuration directly.

It may be possible to have \require collect the config() methods from the dependencies and run them after all the initialization is done, but I didn't want to make that big a change here. This should the job for now.

@dpvc dpvc requested a review from zorkow March 9, 2024 20:32
@dpvc dpvc added this to the v4.0 milestone Mar 9, 2024
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

Question on priority. Otherwise looks fine.

@@ -39,6 +39,7 @@ import './TextMacrosMappings.js';
*/
export const TextBaseConfiguration = Configuration.create('text-base', {
parser: 'text',
priority: 1,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for the priority?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was so that the base configuration is done first and then others that override anything in it don't need to specify their own priorities. But I think that the packages may be processed in reverse order, which means I should make this 10 instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just checked, and this does mean text-base will have its init() and config() methods called before the other configurations. So that means the default priority of 5 will put others after this, without having to set the priority.

@dpvc dpvc merged commit 7a5dbdc into develop Mar 16, 2024
@dpvc dpvc deleted the textcomp-textmacros branch March 16, 2024 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants