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

[#52] Adds a generate command for a default .tools.toml file #62

Merged
merged 11 commits into from
Sep 8, 2022

Conversation

MitchellBerend
Copy link
Collaborator

Resolves #52.

The generate command also uses the --config flag to set the location of the generated file. Would it be a good idea to have some output when actually generating the config? Right now there is no actual response unless there is some file system error.

$ cargo run -- --config test-tools.toml generate
   Compiling tool-sync v0.1.0 (/home/mitchell/rust/tool-sync)
    Finished dev [unoptimized + debuginfo] target(s) in 2.14s
     Running `target/debug/tool --config test-tools.toml generate``

$ cat test-tools.toml 
# This file was automatically generated by tool-sync
#
#store_directory = "$HOME/.tools.toml"
#
#    [bat]
#        owner =     "sharkdp"
#        repo =      "bat"
#        exe_name =  "bat"
#        tag =       "latest"
#    [exa]
#        owner =     "ogham"
#        repo =      "exa"
#        exe_name =  "exa"
#        tag =       "latest"
#    [fd]
#        owner =     "sharkdp"
#        repo =      "fd"
#        exe_name =  "fd"
#        tag =       "latest"
#    [ripgrep]
#        owner =     "BurntSushi"
#        repo =      "ripgrep"
#        exe_name =  "rg"
#        tag =       "latest"

@chshersh chshersh added config TOML configuration, config-related CLI options CLI Command Line Interface output Fancy (and not so) output of the tool labels Sep 3, 2022
@MitchellBerend
Copy link
Collaborator Author

We should wait for the discussion in #65 to be resolved since this issue might constitute changes in this pr

@MitchellBerend
Copy link
Collaborator Author

The file that currently gets generated looks like this

$ cat test-tools.toml 
# This file was automatically generated by tool-sync
#
# The directory to store all tools in
#store_directory = "$HOME/.local/bin"
#
#[ripgrep]
#   owner     = "BurntSushi"
#   repo      = "ripgrep"
#   exe_name  = "rg"
#
#   # Uncomment to download a specific version or tag.
#   # Without this tag latest will be used
#   # tag       = "latest"
#
#
# Asset name to download on linux OSes
#asset_name.linux = "x86_64-unknown-linux-musl"
#
# uncomment if you want to install on macOS as well
#asset_name.macos = "apple-darwin"
#
# uncomment if you want to install on Windows as well
#asset_name.windows = "x86_64-pc-windows-msvc"

Is this missing anything?

@MitchellBerend MitchellBerend marked this pull request as ready for review September 5, 2022 10:15
Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

This is a great change 🏆

I feel like the design space for this innocently-looking option is too big so I'd like to have a conversation. But good defaults matter 👏🏻 So I want users to have the best UX.

src/config/cli.rs Outdated Show resolved Hide resolved
src/config_template.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/config_template.rs Outdated Show resolved Hide resolved
src/config_template.rs Outdated Show resolved Hide resolved
src/config_template.rs Outdated Show resolved Hide resolved
src/config_template.rs Outdated Show resolved Hide resolved
Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Just one minor comment and this PR is ready to go 👍🏻

Really appreciate your work here, @MitchellBerend 👏🏻
The more I think about this, the more ideas and feature requests I can come up with. But let's take time and settle on some design and refine it later.

README.md Show resolved Hide resolved
@MitchellBerend
Copy link
Collaborator Author

MitchellBerend commented Sep 8, 2022

Issues that can be created following this pr
The first 2 could maybe all get the tag `good-first-issue

  • Add a -f|--force flag to the default-config sub command to let it overwrite the existing $HOME/.tools.toml

If the --config flag is specified, the default configuration is saved in the explicitly specified path. If this file already exist, ask interactively whether user want's to change it. And if the answer is "yes" then override it. The defaulg-config command can also accept the -f|--force flag to ignore interactive question.


  • Always require the --config flag with the default-config command

  • Add current version to the generated config without hard coding it in the actual template.

Would be cool to also say by which version of tool-sync but not sure how to do this without hardcoding the version thinking

We can do some magic in a build.rs file that can generate the src/config/template.rs so it contains the version in the config template itself.


Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Thanks a ton!

@chshersh
Copy link
Owner

chshersh commented Sep 8, 2022

@MitchellBerend These new issues look like a good improvement over the existing state of affairs 👍🏻
I'm thinking about a completely alternative design for the config-related commands (e.g. maybe it would be better to have the tool config --default command and other flags to the config command?) so I'll create an RFC soon and I'd like to invite you to the discussion. And when it's decided what interface is better, we can continue with other improvements around configs 🚢

@chshersh
Copy link
Owner

chshersh commented Sep 9, 2022

I've created an issue to discuss the future interface of all config-related commands:

@chshersh chshersh changed the title Adds a generate command for a default .tools.toml file [#52] Adds a generate command for a default .tools.toml file Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command Line Interface config TOML configuration, config-related CLI options output Fancy (and not so) output of the tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a command to output default config with comments and explanations
2 participants