Skip to content

git-config towards 1.0 #191

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

Closed
5 tasks
Tracked by #303
Byron opened this issue Sep 1, 2021 · 5 comments
Closed
5 tasks
Tracked by #303

git-config towards 1.0 #191

Byron opened this issue Sep 1, 2021 · 5 comments

Comments

@Byron
Copy link
Member

Byron commented Sep 1, 2021

A list of ideas for features that come up when using git-config::file::GitConfig

  • Boolean can act as bool or be converted to one, maybe using deref
  • Integer derefs to integer
  • Something like try_value() -> Result<Option<T>, E> as currently I am ignoring all errors as it's too cumbersome to check if the conversion failed which probably should be an actual
  • could there be a more convenient API than this.

Missing features

  • cascading config files
  • know which config files to cascade
  • actual PathBuf instead of just a string
  • comfortable API for accessing values
  • include directive

Related Discussions

@Byron Byron changed the title [git-config] Wishlist [git-config] Wishlist and desired features Oct 10, 2021
@caspervonb
Copy link
Contributor

caspervonb commented Oct 10, 2021

First thoughts; knocking out some tests and the features before even thinking about refactoring or breaking anything but I think we should would avoid doing any magical deref here.
Some as_ helpers returning Option could work pretty when well chained into map/and_then etc.

Parser itself is solid and pretty nice as-is, but it would be nice if we had a more user friendly serde-json like Value structure after parsing for GitConfig so that boolean for example, was simply a Bool(bool).

Currently Value is a mix between fairly standard serve variant type and a non-allocating pull parser token.
Could be nicer if there was an owning structure that was more serde-friendly.

Raw pull parser, its items and events would still be available but am slightly leaning towards an easier to use abstraction on top of it.

@edward-shen
Copy link
Contributor

What would you suggest for a concrete implementation then? A separate type ParserValue and a serde friendly enum? Or just enhancements to the existing Value (e.g. as_*) and to_owed?

Ironically enough, serde compatibility was always a goal from the beginning but never achieved (yet)

@caspervonb
Copy link
Contributor

What would you suggest for a concrete implementation then?

Not quite sure yet, but yes leaning a bit towards a separate variant type ala serde_json::Value, maybe we encode sections into that enum type.

I'll probably sketch something out in a pull request soon.

@Byron
Copy link
Member Author

Byron commented Oct 12, 2021

Following the conversation, I have trouble understanding how serde friendliness is going to help with implementing the missing features. The only connection I see is how values are returned by the API, and maybe making this more comfortable would force changing the parser.

All I hope is that making these adjustments won't turn out to be an unexpectedly major undertaking.

Please note that I am expressing my opinion based on non-neglectable ignorance about the actual implementation, so it might be missing the point as well.

@caspervonb
Copy link
Contributor

Just ergonomics and usability, parser features can be expanded upon just fine without touching or changing much.
Main thing would be, that with serde one could represent configuration as plain types in user-code.

#[derive(Serialize, Deserialize)]
struct CoreSection {
  // ...
}

#[derive(Serialize, Deserialize)]
struct Config {
  core: Option<CoreSection>,
  // ...
}

@Byron Byron changed the title [git-config] Wishlist and desired features git-config towards 1.0 Feb 8, 2022
@Byron Byron added C-tracking-issue An issue to track to track the progress of multiple PRs or issues and removed C-tracking-issue An issue to track to track the progress of multiple PRs or issues labels Feb 8, 2022
@Byron Byron closed this as completed Feb 8, 2022
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

3 participants