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

Putting names in toml hash table headings #45

Open
theherk opened this issue Sep 27, 2022 · 2 comments
Open

Putting names in toml hash table headings #45

theherk opened this issue Sep 27, 2022 · 2 comments

Comments

@theherk
Copy link

theherk commented Sep 27, 2022

I wonder if you see any merit it putting names into using a slightly simplified (I think) configuration. This uses quite a few arrays as is that contain names for things that are already atomic. For example, directory names and remote names already require uniqueness.

In my view, this would make a slightly more terse and readable configuration in any format. For example, this config.toml:

[[trees]]
root = "~/projects"

[[trees.repos]]
name = "github.com/theherk/terraform-aws-apigateway-route-builder"
worktree_setup = false

[[trees.repos.remotes]]
name = "origin"
url = "[email protected]:theherk/terraform-aws-apigateway-route-builder.git"
type = "ssh"

[[trees.repos]]
name = "github.com/theherk/helix"
worktree_setup = false

[[trees.repos.remotes]]
name = "origin"
url = "[email protected]:theherk/helix.git"
type = "ssh"

[[trees.repos.remotes]]
name = "upstream"
url = "https://github.com/helix-editor/helix.git"
type = "https"

would become:

root = "~/projects"

["github.com".theherk.terraform-aws-apigateway-route-builder]
worktree_setup = false

["github.com".theherk.terraform-aws-apigateway-route-builder.remotes.origin]
type = "ssh"
url = "[email protected]:theherk/terraform-aws-apigateway-route-builder.git"

["github.com".theherk.helix]
worktree_setup = false

["github.com".theherk.helix.remotes.origin]
type = "ssh"
url = "[email protected]:theherk/helix.git"

["github.com".theherk.helix.remotes.upstream]
type = "https"
url = "https://github.com/helix-editor/helix.git"

That would have the byproduct of forcing names not be duplicated in these cases, and it feels a bit more nice to me.

Of course, you have put together a wonderful tool. Thank you.


Also, I think this could be further simplified using some very sensible heuristics to get the same configuration from:

root = "~/projects"

["github.com".theherk.terraform-aws-apigateway-route-builder]
worktree_setup = false

["github.com".theherk.helix]
worktree_setup = false

["github.com".theherk.helix.remotes.upstream]
url = "https://github.com/helix-editor/helix.git"
@hakoerber
Copy link
Owner

hakoerber commented Sep 27, 2022

Hey,

I really like the idea! I'm not happy with the current, super verbose TOML config (I'm mainly using YAML for my own stuff, as it's much shorter).

Implementing the change should be possible. To give you a short overview over the code that handles the configuration file: All the config reading & parsing is done in src/config.rs. For the configuration that just lists repositories, see the ConfigTrees struct: It's just a Vec of ConfigTree (duh), which in turn contains a root and a list of RepoConfigs. Those contain a name, the worktree_setup bool and the remotes configuration.

The important function that is used by the rest of the code to get the configuration itself is read_config. It's generic over the returned configuration, but for your change it's only necessary to adapt the code for the ConfigTrees variant of the Config enum.

All the actual parsing is done by serde, which is quite cool in that regard: You can just throw some data types at it for parsing, and it will figure out which one "fits" the data. So if you can encode the new config format in the type system, serde will parse it for you. The only thing that would then be left is to translate it to the current format for further processing.

So I think the easiest way to do this would be the following:

  • Create a struct that describes the new configuration. I'm honestly not sure how to convince serde to parse these nested paths (github.com, username, repo name), but I'm sure it is possible. Something like ConfigTreesV2, ConfigTreeV2 and RepoConfigV2. I think this will be the hardest part.
  • Write an adapter that translates ConfigTreesV2 to ConfigTrees. This would also where you would implement the heuristics to use type = "https" for https:// URLs.
  • Update read_config() to try parsing the configuration file using the new format in case the other formats (the current TOML & YAML) fail. In that case, use the adapter to return a ConfigTrees struct, so the method signature does not change and no callsites in the rest of the code need to be adapter.

This is just a guess without having tried it out. There might be a more elegant way. If you know another way, let me know, I'm always happy to discover better approaches. Regardless of the approach in the end, I think it's a good idea to encapsulate config parsing inside config.rs, so the changes should be limited to that module.

Let me know if you're interested in giving the implementation a shot. Otherwise, I may give it a try (I really like your new format), but I'm quite busy at the moment.

In any case, thank you for suggestion! Have a nice day. :)

@theherk
Copy link
Author

theherk commented Sep 28, 2022

Okay. Very helpful, thorough information. Thank you for the feedback. I suffer from being quite busy at the moment too. However, as soon as I find some spare time I will cut ground on this, but it may be that you or another beat me to it. I think it is possible to support both without any interruption to users based on your guidance. As an aside, the forge implementation is awesome, too.

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

No branches or pull requests

2 participants