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(cli): Improve CLI logging #4060

Merged
merged 30 commits into from
May 7, 2022
Merged

feat(cli): Improve CLI logging #4060

merged 30 commits into from
May 7, 2022

Conversation

JonasKruckenberg
Copy link
Contributor

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

This PR overhauls the CLI's output, completely moving it from a custom logging solution to the standard log and env_logger crates. Towards this goal I removed all printing functionality from the bundler, cleaning up concerns and bringing the bundler one step closer to being a reusable library.

Major new feature

  • The CLI now offers way more debug output, logging parameters and outputs of each executed command and shell script plus more.
  • The produced output now look much closer to cargo's output (and IMO better than before)
    For this we leverage a custom KV attribute for log macros: action that will take the log levels position in the output. info!(action = "Bundling"; "{}", app_product_name); produces Bundling <App Name>
  • The log level can be controlled via the --verbose flag and the RUST_LOG env var. (The flag can appear multiple times, where --verbose means Debug and --verbose --verbose means Trace)

@JonasKruckenberg JonasKruckenberg requested review from a team May 4, 2022 19:55
@JonasKruckenberg JonasKruckenberg requested a review from a team as a code owner May 4, 2022 19:57
@JonasKruckenberg
Copy link
Contributor Author

I think this doesn't compile on linux or windows 🤔 but my rust-analyzer seems to be broken rn

@JonasKruckenberg JonasKruckenberg changed the title Improve CLI logging feat(cli): Improve CLI logging May 5, 2022
@JonasKruckenberg JonasKruckenberg requested a review from a team May 5, 2022 11:45
Copy link
Member

@lucasfernog lucasfernog left a comment

Choose a reason for hiding this comment

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

top5 best Tauri PRs easily

tooling/bundler/src/bundle/settings.rs Show resolved Hide resolved
@amrbashir
Copy link
Member

amrbashir commented May 6, 2022

Bug alert, on Windows, if you have before(Build|Dev)Command, it prints the before(Build|Dev)Command output then hangs indefinitely and if before(Build|Dev)Command clears the terminal like how vite devServer does by default, it will only print Running BeforeDevCommand (pnpm dev).

@JonasKruckenberg
Copy link
Contributor Author

JonasKruckenberg commented May 6, 2022

Bug alert, on Windows, if you have before(Build|Dev)Command, it prints the before(Build|Dev)Command output then hangs indefinitely and if before(Build|Dev)Command clears the terminal like how vite devServer does by default, it will only print Running BeforeDevCommand (pnpm dev).

Is that using the latest commit from @lucasfernog? Maybe it has to do with the piping? I had a bunch of issues while developing with disappearing output too.

Maybe not though, since it's the same implementation as before basically.

@lucasfernog do we need os_pipe in the first place? We could just use the standard Stdio::inherit() if don't need the output.

@lucasfernog
Copy link
Member

We do need the os_pipe @JonasKruckenberg otherwise the binding just won't show the output.

@lucasfernog
Copy link
Member

@amrbashir that was fixed in my commits.

@amrbashir
Copy link
Member

amrbashir commented May 6, 2022

Oh then I might've checked with an older commit. I tested the branch twice and I missed a git pull in between.

Edit: just tested latest commits and it works.

@lucasfernog lucasfernog merged commit 35f2147 into dev May 7, 2022
@lucasfernog lucasfernog deleted the verbose-codesign-output branch May 7, 2022 13:19
@JonasKruckenberg
Copy link
Contributor Author

Niiice 🥳

dceddia pushed a commit to dceddia/tauri that referenced this pull request May 14, 2022
Co-authored-by: Lucas Nogueira <lucas@tauri.studio>
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.

3 participants