Skip to content

V2 CLI: separate new and init commands - #983

Merged
pkgw merged 6 commits into
tectonic-typesetting:masterfrom
caiogeraldes:master
Jan 24, 2023
Merged

V2 CLI: separate new and init commands#983
pkgw merged 6 commits into
tectonic-typesetting:masterfrom
caiogeraldes:master

Conversation

@caiogeraldes

Copy link
Copy Markdown
Contributor

This is a simple quality of life change I would like to suggest, as it standardizes the CLI of tectonic with that of tools such as cargo or poetry. Not much else to say, tbh.

separates 'new' and 'init' commands

separates 'new' and 'init' commands
@caiogeraldes caiogeraldes changed the title V2 CLI: separates new and `init commands V2 CLI: separates new and init commands Dec 30, 2022
@pkgw

pkgw commented Jan 7, 2023

Copy link
Copy Markdown
Collaborator

Thank you for the submission! This seems like a nice improvement to me. I have two requests:

  1. Can you please add a commit fix the clippy error that the CI is flagging? It's unrelated to your change but also should be a trivial fix. (Sometimes when new versions of Rust come out, we start getting new errors without changing our code, because Clippy gets stricter.)
  2. Can you please update the documentation as well? The files are in docs/src/v2cli/ and docs/src/SUMMARY.md.

Thanks again!

@pkgw pkgw changed the title V2 CLI: separates new and init commands V2 CLI: separate new and init commands Jan 7, 2023
@caiogeraldes

caiogeraldes commented Jan 15, 2023

Copy link
Copy Markdown
Contributor Author

The last two commits should hopefully fix the warnings clippy was throwing, most of them concerning the pattern variable_X as type_Y.

However, I could not fix one of them without setting #[allow(clippy::write_literal)] in the following lines:

tests/executable.rs, l.741ff.

        #[allow(clippy::write_literal)]
        writeln!(
            file,
            "{}", // <= works around {} fussiness in Rust format strings
            r#"\newwrite\w
\immediate\openout\w=first.demo\relax
\immediate\write\w{content-un}
\immediate\closeout\w
\immediate\openout\w=second.demo\relax
\immediate\write\w{content-deux}
\immediate\closeout\w
"#

Without it, clippy went into a kind of a loop, asking for more and more curly braces around content-un and content-deux. Considering it is part of a test which is passing, I assumed the warning could be ignored without any issues. I think the issue is with clippy in this case, and I reckon that the comment near "{}", is related.

Edit:
It seems that a new error appeared while building the version for the target x86_64-pc-windows-msvc with vcpkg. I believe it is unrelated to my changes.

@codecov

codecov Bot commented Jan 15, 2023

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@abd98d4). Click here to learn what that means.
The diff coverage is 46.42%.

❗ Current head 92261fa differs from pull request most recent head cf9e093. Consider uploading reports for the commit cf9e093 to get more accurate results

@@            Coverage Diff            @@
##             master     #983   +/-   ##
=========================================
  Coverage          ?   45.17%           
=========================================
  Files             ?      148           
  Lines             ?    62159           
  Branches          ?        0           
=========================================
  Hits              ?    28083           
  Misses            ?    34076           
  Partials          ?        0           
Impacted Files Coverage Δ
crates/xdv/src/lib.rs 0.00% <0.00%> (ø)
src/bin/tectonic/v2cli.rs 46.86% <13.33%> (ø)
crates/bundles/src/cache.rs 79.31% <50.00%> (ø)
crates/bridge_flate/src/lib.rs 33.89% <100.00%> (ø)
src/config.rs 58.97% <100.00%> (ø)
src/docmodel.rs 55.95% <100.00%> (ø)
src/driver.rs 76.17% <100.00%> (ø)
src/io/format_cache.rs 88.23% <100.00%> (ø)
src/test_util.rs 88.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pkgw pkgw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! There's one small thing to fix in the new docs.

As for the Clippy stuff, thanks — when I initially asked there was only one small error, so you ended up doing a lot more work than I wanted to ask of you!

I'll look into the MSVC issue. I think it's something that might not be too hard to fix/work around. I might fix that doc title issue while I'm at it.

Comment thread docs/src/v2cli/init.md Outdated
@pkgw
pkgw merged commit b4701f5 into tectonic-typesetting:master Jan 24, 2023
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