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

feat(quarto): Add Quarto module #5820

Merged
merged 1 commit into from Mar 20, 2024
Merged

feat(quarto): Add Quarto module #5820

merged 1 commit into from Mar 20, 2024

Conversation

Armavica
Copy link
Contributor

@Armavica Armavica commented Mar 3, 2024

Description

I added a module for the Quarto publishing system on the model of the Typst module.

Motivation and Context

Screenshots (if appropriate):

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@Armavica Armavica force-pushed the quarto branch 2 times, most recently from 1b49064 to 00cb5b1 Compare March 3, 2024 19:49
})
.map(|variable| match variable {
"version" => {
let version = context.exec_cmd("quarto", &["--version"])?.stdout;
Copy link
Member

Choose a reason for hiding this comment

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

Please handle the trailing newline and update the command mocks to add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, done.

| Variable | Example | Description |
| ------------- | --------- | ------------------------------------------------- |
| version | `1.4.549` | The version of `quarto`, alias for quarto_version |
| typst_version | `default` | The current Typst version |
Copy link
Contributor

@jankatins jankatins Mar 3, 2024

Choose a reason for hiding this comment

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

I don't see anything parsing/filling that variable. Is this needed/wanted?

In any case: given that quarto bundles typst, what's the value in this, given that in most (or even all cases: can you overwrite the typst version by installing a newer version), the quarto version will determine the typst version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, that's a copy/paste fail. I removed the variable.

@Armavica Armavica force-pushed the quarto branch 4 times, most recently from 1f26e89 to a3107aa Compare March 4, 2024 16:17
@jankatins
Copy link
Contributor

The commit and PR message is still mentioning Typst

@Armavica
Copy link
Contributor Author

Armavica commented Mar 4, 2024

The commit and PR message is still mentioning Typst

It was to credit the Typst module that I copy/pasted and adapted for this PR. But I can remove this mention if you want.

@davidkna davidkna changed the title feat: Add Quarto module on the model of Typst feat: Add Quarto module Mar 10, 2024
@davidkna
Copy link
Member

LGTM, but please fix the conflicts.

* Adapted from the Typst module
@Armavica
Copy link
Contributor Author

@davidkna Done!

@andytom andytom changed the title feat: Add Quarto module feat(quarto): Add Quarto module Mar 20, 2024
@andytom andytom merged commit 0e49f04 into starship:master Mar 20, 2024
21 checks passed
@andytom
Copy link
Member

andytom commented Mar 20, 2024

Thank you for your contribution @Armavica and thank you for reviewing @davidkna.

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

4 participants