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

Refactor commands into separate modules #94

Merged
merged 4 commits into from
Sep 16, 2022

Conversation

chshersh
Copy link
Owner

Some refactoring to change a few things:

  • Move all commands into separate modules to make the top-level match in lib.rs concise
  • Change the text of the default-config
  • Refactor formatting of tool names into a separate function
  • Rename integration test files to specify whether they are used for the the sync or install command

Comment on lines +121 to +127
pub fn fmt_tool_names<F: FnMut(&String) -> String>(fmt_tool: F) -> String {
build_db()
.keys()
.map(fmt_tool)
.collect::<Vec<String>>()
.join("\n")
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

I wanted to keep the previous implementation with writeln! but wasn't able to specify the proper function type signature so I went for a simpler implementation. It is probably less efficient but it looks pretty simple and the performance overhead is not noticeable actually. It still works instantaneously.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right join actually makes sense here. I was looking for a way to concatenate strings like in python, turns out it is the same...

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's a bit sad it doesn't work with iterators and I need to collect first. There's itertools which implements this. But I didn't want to add an extra crate for a minor convenience.

Moreover, according to this comment, join on Vec is even faster than from itertools because it can allocate the required amount of memory.

The comment is 5 years old so things could've changed since then

# Without this tag latest will be used
# tag = "13.0.0"

# Asset name to download on linux OSes
Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not realize these options were not global in the config. I guess it does make sense since not all tools will support musl or libc or windows.

@MitchellBerend
Copy link
Collaborator

Maybe this is just me but I like having the line start with the #, this is obviously preference though.

# This configuration is automatically generated by tool-sync 0.1.0
# https://github.com/chshersh/tool-sync
#######################################
#
# Installation directory for all the tools:
# store_directory = "$HOME/.local/bin"
#
# tool-sync provides native support for some of the tools without the need to
# configure them. Uncomment all the tools you want to install with a single
# 'tool sync' command:
#
# [bat]
# [difftastic]
# [exa]
# [fd]
# [ripgrep]
# [tool-sync]
#
# You can configure the installation of any tool by specifying corresponding options:
#
# [ripgrep]  # Name of the tool (new or one of the hardcoded to override default settings)
#     owner     = "BurntSushi"  # GitHub repository owner
#     repo      = "ripgrep"     # GitHub repository name
#     exe_name  = "rg"          # Executable name inside the asset

#     Uncomment to download a specific version or tag.
#     Without this tag latest will be used
#     tag       = "13.0.0"

#     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"

Comment on lines +38 to +48
### START ###

# ensure this directory is listed in $PATH
store_directory = "/path/to/install/directory"

[bat]
[exa]
[fd]
[ripgrep]

[bat]
[exa]
[fd]
[ripgrep]
### END ###
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good change actually, this makes it more clear where the config starts and ends.

Comment on lines +121 to +127
pub fn fmt_tool_names<F: FnMut(&String) -> String>(fmt_tool: F) -> String {
build_db()
.keys()
.map(fmt_tool)
.collect::<Vec<String>>()
.join("\n")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right join actually makes sense here. I was looking for a way to concatenate strings like in python, turns out it is the same...

@chshersh
Copy link
Owner Author

Maybe this is just me but I like having the line start with the #, this is obviously preference though.

I don't might changing the format actually. I don't have strong feelings about one or another. Having all the comments as leading comments makes it look more consistent 👍🏻

@chshersh chshersh merged commit 65b53bd into main Sep 16, 2022
@chshersh chshersh deleted the chshersh/organisational-refactoring branch September 16, 2022 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants