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

Custom option #379

Closed
wants to merge 1 commit into from
Closed

Custom option #379

wants to merge 1 commit into from

Conversation

jlaurens
Copy link
Contributor

@jlaurens jlaurens commented Jul 8, 2024

Actually, custom options are documented but not supported.
This PR fixes this problem.

New options must be declared prior to parsing the arguments, it is now done in a new preparation script name build-pre.lua near build.lua eventually loaded by l3build-arguments.lua.

New global function declare_option(⟨name ⟩,⟨table ⟩) is used to declare a new option whereas update_option(⟨name ⟩,⟨other table ⟩,⟨provide ⟩) is used to update option data. Both validate their arguments.

Testing is not very well designed and should be integrated in the check target process, a forthcoming feature.

@jlaurens jlaurens force-pushed the custom_option branch 2 times, most recently from 221e5fd to 8836641 Compare July 8, 2024 15:21
@josephwright
Copy link
Member

This all looks rather baroque: I am wondering if the best plan is simply to rule out custom options, which don't really work with the structure as it is now (they might have done in the older approach).

@davidcarlisle
Copy link
Member

davidcarlisle commented Jul 9, 2024

I don't think there is really any need for custom options, there are enough option names for any particular project and in practice enough hooks that a package needing some custom code can redefine one of the existing options to run whatever code is needed. Or if something really new comes up a package author could make a PR to add a new option to the base l3build code.

@FrankMittelbach
Copy link
Member

agreed, this is a leftover approach and should be dropped from the documentation rather than implemented.

@josephwright
Copy link
Member

Ok, so the consensus is to drop the idea from the docs. I’ll leave the PR open until I can do that.

@jlaurens jlaurens closed this Jul 10, 2024
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.

4 participants