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

Add sys.target export targets #4385

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tingerrr
Copy link
Contributor

@tingerrr tingerrr commented Jun 12, 2024

This PR adds a sys.target, a string variable containing pdf, svg or raster depending on the export target.

It should be noted that, with this PR in its current stage, compilation is run 3 times per average test on --extended. It may be more sensible, but less intuitive to contributors, if we assume a default target of raster and then only override the target for those test which have an appropriate annotation.

TODO

  • allow custom export targets?
  • finalize design for CLI interface
  • write documentation
  • allow contextual override

Related to #4174, partially resolves it.

@tingerrr tingerrr marked this pull request as draft June 12, 2024 15:44
@tingerrr tingerrr changed the title Tingerrr/kprptzstusot Add sys.target export targets Jun 12, 2024
@laurmaedje
Copy link
Member

Relevant prior art: #1904

@tingerrr tingerrr force-pushed the tingerrr/kprptzstusot branch 3 times, most recently from 2a37969 to 84ea4e0 Compare June 12, 2024 18:31
@tingerrr
Copy link
Contributor Author

I have renamed --format on query to --output-format to avoid the clash with --format while maintaining a consistent CLI.

@tingerrr tingerrr force-pushed the tingerrr/kprptzstusot branch 2 times, most recently from 442a179 to 24bbf33 Compare June 13, 2024 13:32
@tingerrr
Copy link
Contributor Author

I've changed the test runner to have three options, one for each export target, running only those that are selected. By default, --raster is implied, but when another export target is requested --raster no longer is. --extended was changed to --all to reflect this change in behavior.

The README contains a note explaining that only render tests are compared.

@laurmaedje laurmaedje added the waiting-on-review This PR is waiting to be reviewed. label Jul 7, 2024
@laurmaedje laurmaedje added waiting-on-design This PR or issue is blocked by design work. and removed waiting-on-review This PR is waiting to be reviewed. labels Jul 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-design This PR or issue is blocked by design work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants