Use Typst's JSON dep file (with outputs) #22
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR updates the compile_typst command to use Typst’s JSON-format dependency files (including both inputs and outputs), adds support for HTML outputs, and bumps required Typst version to 0.14, with corresponding test and documentation updates. Entity relationship diagram for Typst dependency file format changeerDiagram
DEP_FILE_JSON {
string[] inputs
string[] outputs
}
COMPILE_TYPST {
uses DEP_FILE_JSON
}
COMPILE_TYPST ||--|| DEP_FILE_JSON : reads
Class diagram for updated Typst compilation logicclassDiagram
class compile_typst {
+main(argv, work_thread)
-args: Namespace
-typargs: list
-path_dep: Path
+filter_dependencies(inp_paths)
+amend(inp)
}
class WorkThread {
+runsh(command)
}
compile_typst --> WorkThread : uses
compile_typst : +support for .html output
compile_typst : +uses JSON dep file (inputs, outputs)
compile_typst : -removes legacy dep file parsing
compile_typst : +parses depinfo["inputs"], depinfo["outputs"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Here's the code health analysis summary for commits Analysis Summary
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider using Path.with_suffix('.dep.json') instead of slicing off 4 characters for building the dep file path to avoid hard‐coding the extension length.
- You’re parsing both inputs and outputs from the JSON deps file but only using inputs; make sure to handle or surface out_paths for multi-output use cases or remove the misleading comment.
- The CI is currently pinned to the 0.14-rc2 release—either switch to the stable 0.14 release or add a runtime version check to guard against RC-only flags.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using Path.with_suffix('.dep.json') instead of slicing off 4 characters for building the dep file path to avoid hard‐coding the extension length.
- You’re parsing both inputs and outputs from the JSON deps file but only using inputs; make sure to handle or surface out_paths for multi-output use cases or remove the misleading comment.
- The CI is currently pinned to the 0.14-rc2 release—either switch to the stable 0.14 release or add a runtime version check to guard against RC-only flags.
## Individual Comments
### Comment 1
<location> `stepup/reprep/compile_typst.py:99-100` </location>
<code_context>
- out_paths.extend(shlex.split(dep_out))
- inp_paths.extend(shlex.split(dep_inp))
+ depinfo = json.load(fh)
+ inp_paths = depinfo["inputs"]
+ out_paths = depinfo["outputs"]
else:
print(f"Dependency file not created: {path_dep}.", file=sys.stderr)
</code_context>
<issue_to_address>
**suggestion:** No error handling for malformed dependency JSON.
Currently, if the dependency file is not valid JSON or lacks 'inputs' or 'outputs', an exception will be raised. Please add error handling to manage these cases and provide informative error messages.
</issue_to_address>
### Comment 2
<location> `stepup/reprep/api.py:349` </location>
<code_context>
if not path_typ.endswith(".typ"):
raise ValueError(f"The input of the typst command must end with .typ, got {path_typ}.")
- path_out = make_path_out(path_typ, dest, ".pdf", [".svg", ".png"])
+ path_out = make_path_out(path_typ, dest, ".pdf", [".svg", ".png", ".html"])
stem = path_typ[:-4]
</code_context>
<issue_to_address>
**suggestion:** Consider centralizing supported output formats.
Duplicating the list increases the chance of inconsistencies and makes updates harder. Centralizing it will simplify future changes.
Suggested implementation:
```python
path_out = make_path_out(path_typ, dest, ".pdf", SUPPORTED_TYPST_OUTPUT_FORMATS)
```
```python
SUPPORTED_TYPST_OUTPUT_FORMATS = [".svg", ".png", ".html"]
(Referenced bitmaps are handled correctly.)
These referenced SVG figures are not rendered, neither are they included in the dep file.
See: https://github.com/typst/typst/issues/6858
Parameters
----------
```
</issue_to_address>
### Comment 3
<location> `stepup/reprep/compile_typst.py:53` </location>
<code_context>
def main(argv: list[str] | None = None, work_thread: WorkThread | None = None):
"""Main program."""
args = parse_args(argv)
if work_thread is None:
work_thread = WorkThread("stub")
if not args.path_typ.endswith(".typ"):
raise ValueError("The Typst source must have extension .typ")
if not (args.path_out is None or args.path_out.suffix in (".pdf", ".png", ".svg", ".html")):
raise ValueError("The Typst output must be a PDF, PNG, SVG, or HTML file.")
# Get Typst executable and prepare some arguments that
if args.typst is None:
args.typst = getenv("REPREP_TYPST", "typst")
# Prepare the command to run Typst
typargs = [args.typst, "compile", args.path_typ]
if args.path_out is not None:
typargs.append(args.path_out)
else:
args.path_out = Path(args.path_typ[:-4] + ".pdf")
if args.path_out.suffix == ".png":
resolution = args.resolution
if resolution is None:
resolution = int(getenv("REPREP_TYPST_RESOLUTION", "144"))
typargs.append(f"--ppi={resolution}")
elif args.path_out.suffix == ".html":
typargs.append("--features=html")
for keyval in args.sysinp:
typargs.append("--input")
typargs.append(keyval)
if len(args.typst_args) == 0:
args.typst_args = shlex.split(getenv("REPREP_TYPST_ARGS", ""))
typargs.extend(args.typst_args)
with contextlib.ExitStack() as stack:
if args.keep_deps:
# Remove any existing make-deps output from a previous run.
path_dep = Path(args.path_typ[:-4] + ".dep.json")
path_dep.remove_p()
else:
# Use a temporary file for the make-deps output.
path_dep = stack.enter_context(TempDir()) / "typst.dep.json"
typargs.extend(["--deps", path_dep, "--deps-format", "json"])
# Run typst compile
returncode, stdout, stderr = work_thread.runsh(shlex.join(typargs))
print(stdout)
# Assume there is a single output file, which is the one specified.
# This is not correct when there are multiple outputs, e.g. as with SVG and PNG outputs.
# Get required input files from the dependency file.
if path_dep.is_file():
with open(path_dep) as fh:
depinfo = json.load(fh)
inp_paths = depinfo["inputs"]
out_paths = depinfo["outputs"]
else:
print(f"Dependency file not created: {path_dep}.", file=sys.stderr)
out_paths = []
inp_paths = []
sys.stderr.write(stderr)
inp_paths = filter_dependencies(inp_paths)
amend(inp=inp_paths)
# Write inventory
if args.inventory is not None:
inventory_paths = sorted(inp_paths) + out_paths
write_inventory(args.inventory, inventory_paths, do_amend=False)
# If the output path contains placeholders `{p}`, `{0p}`, or `{t}`,
# we need to amend the output.
if any(p in args.path_out for p in ("{p}", "{0p}", "{t}")):
amend(out=out_paths)
if returncode != 0:
# Only use sys.exit in cases of an error,
# so other programs may call this function without exiting.
sys.exit(returncode)
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Simplify logical expression using De Morgan identities ([`de-morgan`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/de-morgan/))
- Use f-string instead of string concatenation [×2] ([`use-fstring-for-concatenation`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-fstring-for-concatenation/))
- Merge consecutive list appends into a single extend ([`merge-list-appends-into-extend`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/merge-list-appends-into-extend/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
stepup/reprep/compile_typst.py
Outdated
Comment on lines
99
to
100
| inp_paths = depinfo["inputs"] | ||
| out_paths = depinfo["outputs"] |
There was a problem hiding this comment.
suggestion: No error handling for malformed dependency JSON.
Currently, if the dependency file is not valid JSON or lacks 'inputs' or 'outputs', an exception will be raised. Please add error handling to manage these cases and provide informative error messages.
Tested with typst commit 64641d687fcd658c75fee84bd5d40f9bb7f8404f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch shows how to use a feature proposed in the following issue and PR of Typst:
--deps-format jsonto include output files typst/typst#7194To do:
tile_pdffeature and add an equivalent example with Typst