Skip to content

Conversation

@etiennecollin
Copy link
Contributor

Context

I use a typst template which is stored somewhere on my computer. I therefore need to update the typst root with the --root argument when I compile or watch a typst file. I needed a way to pass extra arguments to typst through the python package.

Solution

I updated/improved the parsing of the CLI arguments.

  • The parsing is now done with the argparse python package
    • This enables a more robust and complex parsing if needed
    • The CLI now features a help menu
    • The CLI is now arguably more easily maintainable
  • There is added support for extra arguments that are passed to the typst compiler

Changes were tested locally on my side and everything works properly.

Changes

The way to execute commands was changed a bit. The README has been updated accordingly. Here is the new CLI:

> python -m typst_pyimage -h

usage: typst_pyimage [-h] [-c | -w] [-a EXTRA_ARGS] filepath

Typst extension, adding support for generating figures using inline Python code.

positional arguments:
  filepath              The path to the typst file to be compiled/watched

options:
  -h, --help            show this help message and exit
  -c, --compile         Compile the typst file
  -w, --watch           Watch the typst file
  -a EXTRA_ARGS, --extra-args EXTRA_ARGS
                        Extra arguments to be passed to typst

@etiennecollin
Copy link
Contributor Author

etiennecollin commented Mar 26, 2024

In any case, thank you for this great plugin!

One thing I want to check with you is that to support extra args that contain a ~ such as --extra-args "--root ~", I added a replace() call to the extra args replacing ~ with os.path.expanduser("~") which returns the home environment variable of the current user. This call to os.path.expanduser() is compatible with both unix and windows environments.

This could cause issues if a user uses the ~ symbol in an extra argument and does not want it to expand to their home directory, but I believe such cases should be rare enough... If this replace() is not there, the expansion is not made and the compilation fails.

I could first split the arguments and only do the replacement in the entry following ["--root"], but this reduces the maintainability of the code in that other args than --root could require a path. Let me know if this is something that you think is desirable/necessary.

See:

extra_args = args.extra_args.replace("~", os.path.expanduser("~")).split()

Copy link
Owner

@patrick-kidger patrick-kidger left a comment

Choose a reason for hiding this comment

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

Nit aside this looks awesome to me! Thank you for contributing this.
Do you want to bump the version number in pyproject.toml as well? Then we can do a new release with this functionality.

@patrick-kidger
Copy link
Owner

One thing I want to check with you is that to support extra args that contain a ~ such as --extra-args "--root ", I added a replace() call to the extra args replacing ~ with os.path.expanduser("") which returns the home environment variable of the current user. This call to os.path.expanduser() is compatible with both unix and windows environments.

I think the current implementation seems good enough for now! If use-cases emerge for needing something else then we can tweak things then.

@etiennecollin
Copy link
Contributor Author

etiennecollin commented Mar 27, 2024

@patrick-kidger Since we are introducing breaking changes to the CLI commands (moving from typst_pyimage compile/watch to typst_pyimage --compile/--watch ), should we bump the version to 0.1.0? What do you think?

Breaking changes to the CLI commands.
Moving from `typst_pyimage compile/watch` to `typst_pyimage --compile/--watch`.
@patrick-kidger
Copy link
Owner

I just got a notification about this -- sorry, it looks like I missed your updates to this one!

Since you mention it, why change from e.g. compile to --compile, actually? argparse can be used to run subcommands just as well.

(Either way I think the README.md currently mentions both.)

@etiennecollin
Copy link
Contributor Author

etiennecollin commented May 1, 2024

I just got a notification about this -- sorry, it looks like I missed your updates to this one!

Since you mention it, why change from e.g. compile to --compile, actually? argparse can be used to run subcommands just as well.

(Either way I think the README.md currently mentions both.)

No worries, life happens!

You are right, I found a way to make things cleaner with subcommands. Let me know what you think.

@patrick-kidger
Copy link
Owner

I think it looks good! I have no comments -- if you can fix the pre-commit failures (probably just a matter of pre-commit install; pre-commit run --all-files; git add *; git commit, see here) then I'd be very happy to merge this as-is.

@etiennecollin
Copy link
Contributor Author

I think it looks good! I have no comments -- if you can fix the pre-commit failures (probably just a matter of pre-commit install; pre-commit run --all-files; git add *; git commit, see here) then I'd be very happy to merge this as-is.

Should be good to go! Thanks!

@patrick-kidger patrick-kidger merged commit 50b3a30 into patrick-kidger:main May 8, 2024
@patrick-kidger
Copy link
Owner

Wonderful stuff! Merged. Thank you for your efforts -- and for your patience! -- on this one :D

@etiennecollin
Copy link
Contributor Author

Wonderful stuff! Merged. Thank you for your efforts -- and for your patience! -- on this one :D

No worries! Thank you for your great package!

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.

2 participants