bin
bin copied to clipboard
marcosnils
WIP: Feature/add config cmd
This PR is a concept for introducing a config command. I think it would be a nice feature to modify the configuration via a dedicated command. For example to set up the default path without interacting with the command line (scripted).
In my case, for example, I would use that to set up bin as part of my dotfiles-setup:
> bin config set default_path /home/user/.local/bin
> bin install ….
In this concept, the config-struct is accessed via reflection to make it easy to add/remove config fields later without maintaining a list of available fields.
During the implementation, I came to the conclusion, that it would be way easier (nicer) if the configuration file and the bin-database would be in separate files. Also in view of the fact that the configuration could become more extensive overtime. Otherwise, the database would have to be considered separately in any case, because in my opinion, it would not make sense, for example, to change the hash value of an entry. So I separated the files and implemented a check, whether there is already a configuration, and convert it if that’s the case.
Implemented so far:
bin config set <name> <value>bin config get <name>
Thx for the contribution @rverst , looks a very interesting feature indeed.
Even though I find it interesting doesn't seem to me that bin "needs" this right away. There's only 1 configuration setting today and that's something that users won't usually change. I personally prefer adding features when they're really necessary, and it seems to be that we don't really need this right now.
I'd like to leave this open since it might become useful in the future, but my take on that is to keep feature set small until it's really necessary. Since we don't know what's gonna ultimate happen, we might not ever merge this as well.
I'd like to hear other's opinion as well.. cc @sirlatrom @breml
Additionally, why do we need reflection for this?
Since Go can naturally marshall / unmarshall jsons intro map[string]interface{}, seems like we could be doing this without reflect, right?
i.e: https://play.golang.org/p/gGwVrDqxZMB
I like the separation of config and bin-database, because it really feels like two different things.
I like the separation of config and bin-database, because it really feels like two different things.
If that's the goal then I suggest a new PR for that, using XDG_DATA_HOME for the database part. We could consider using https://github.com/adrg/xdg for cross-platform support.
SGTM. Still, I don't think merging this makes a lot of sense though given the fact that we only have 1 configuration property in the config file so far.
Additionally, why do we need reflection for this?
Since Go can naturally marshall / unmarshall jsons intro
map[string]interface{}, seems like we could be doing this without reflect, right?
I used reflection because I wanted to base that on the config structure, not on the json file. However, there are probably many ways to address the problem.
SGTM. Still, I don't think merging this makes a lot of sense though given the fact that we only have 1 configuration property in the config file so far.
I don't get this point, what would be the number of configuration options needed to justify this feature? In my opinion it is always nice if a cli tool like bin can be used in scripts without causing problems with user prompts, e.g. which path to use as default if it is the first use of the tool. Even if the first install command explicitly includes the path, but that's a different issue.
I do have a second possible configuration option, e.g. the environment variable GITHUB_AUTH_TOKEN might conflict for some users and it would be nice to have such things in the configuration as well.
As a comment to this: If I want to install bin automatically without user interaction this config command would be helpful as I could use this to prevent an interactive prompt showing uo
If I want to install bin automatically without user interaction this config command would be helpful as I could use this to prevent an interactive prompt showing uo
echo '{"defaut_path": "$PATH"}' > $XDG_HOME/.bin/config.json
You can still achieve this today without needing this command. I still don't see the need for a single configuration option at this moment.
echo '{"defaut_path": "$PATH"}' > $XDG_CONFIG_HOME/bin/config.json would be better.