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.

Sign up for GitHub

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

Jump to bottom

config: add workaround for config path expression parsing #3736

Merged
merged 2 commits into from
May 23, 2024
Merged

config: add workaround for config path expression parsing #3736

merged 2 commits into from
May 23, 2024

Conversation

yuja
Copy link
Collaborator

@yuja yuja commented May 22, 2024

#1723

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

yuja added 2 commits May 23, 2024 00:00
@yuja
config: introduce newtype for dotted config name path
dbfb377 
"config list NAME" argument is now parsed as TOML key, but it's still broken
since config.get() expects a query expression in different syntax.

The other config commands will be migrated later.
@yuja
config: add workaround for config path expression parsing
bab0ca6 
As of config 0.13.4, the path Expression type is private, and there's no escape
syntax. This patch adds a fallback to nested HashMap<String, Value> lookup.

https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/mod.rs#L10
https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/parser.rs

Fixes martinvonz#1723
martinvonz
martinvonz approved these changes May 22, 2024
View reviewed changes
Copy link
Owner

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm not sure I remember correctly but I think our plan is still to not use the config crate. One of the benefits of that crate is that it supports multiple formats but we only support TOML anyway.

@yuja yuja merged commit 82b0e88 into martinvonz:main May 23, 2024
16 checks passed
@yuja yuja deleted the push-wtpxwsssrvyl branch May 23, 2024 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinvonz martinvonz martinvonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yuja @martinvonz