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

build(deps): bump toml from 0.5.10 to 0.6.0 #5656

Merged
merged 4 commits into from
Jan 24, 2023

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jan 23, 2023

Bumps toml from 0.5.10 to 0.6.0.

Commits
  • cf5b7d0 chore: Release
  • bd4cd85 chore: Release
  • 607c06b docs: Update changelog
  • b3e0389 Merge pull request #487 from epage/fix
  • 2069c72 docs(toml): Help people serialize/deserialize values
  • 44d2ee8 docs(toml): Update stray reference to Value
  • eb2f20a docs(toml): Remove ignore warning icon
  • a4fd0b1 docs(toml): Update map documentation
  • 709dd35 docs(toml): Mirror serde_json in inlining in root
  • 226639c fix(toml)!: toml! now generates a Table
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added A-dependencies Area: Dependency rust Pull requests that update Rust code labels Jan 23, 2023
@the-mikedavis
Copy link
Member

https://github.com/toml-rs/toml/blob/main/crates/toml/CHANGELOG.md#060---2023-01-23

toml::from_slice has been removed causing some breakages. They recommend toml::from_str

@pascalkuthe
Copy link
Member

#5659 covers the changes necessary for the migration

@pascalkuthe
Copy link
Member

This is a rather large update with significant improvements. See https://epage.github.io/blog/2023/01/toml-vs-toml-edit/ for details.

gibbz00 added a commit to gibbz00/helix that referenced this pull request Jan 24, 2023
@the-mikedavis
Copy link
Member

@dependabot rebase

@dependabot dependabot bot force-pushed the dependabot/cargo/toml-0.6.0 branch from 0c1cb40 to f2c7c00 Compare January 24, 2023 16:09
@the-mikedavis the-mikedavis force-pushed the dependabot/cargo/toml-0.6.0 branch 2 times, most recently from 8b0c916 to 13ec04b Compare January 24, 2023 17:20
dependabot bot and others added 3 commits January 24, 2023 11:21
Bumps [toml](https://github.com/toml-rs/toml) from 0.5.10 to 0.6.0.
- [Release notes](https://github.com/toml-rs/toml/releases)
- [Commits](toml-rs/toml@toml-v0.5.10...toml-v0.6.0)

---
updated-dependencies:
- dependency-name: toml
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
`toml::from_slice` has been removed. The CHANGELOG recommends using
`toml::from_str` instead and doing the byte-to-str conversion yourself.

The `toml::toml!` macro has also changed to return the type of the
value declared within the macro body. In the change in
`helix-view/src/theme.rs` this is a `toml::map::Map` (it was a
`toml::Value` previously) allowing us to skip the match and use the
map directly.

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
The `From<Value>` implementation for `Theme` converted the Value to a
string and re-parsed the string to convert it to
`HashMap<String, Value>` which feels a bit wasteful. This change uses
the underlying `toml::map::Map` directly when the value is a table and
warns about the unexpected `Value` shape otherwise.

This is necessary because toml 0.6.0 changes the Display implementation
for Value::Table so that the `to_string` no longer encodes the value as
a Document, just a Value. So the parse of the Value fails to be decoded
as a HashMap.

The behavior for returning `Default::default` matches the previous
code's behavior except that it did not warn when the input Value was
failed to parse.
@the-mikedavis
Copy link
Member

@pascalkuthe I folded the change from #5671 into this branch. Could you give the changes a second look here? There were some other changes besides from_slice that needed a bit of refactoring

@pascalkuthe
Copy link
Member

I aslo noticed aswell while trying this out locally. This toml release was more breaking than I anticipated. I am already working on it

Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

This toml update fixes #4241

The Theme changes LGTM. It's even an improvement the roundtrip trough string was weird. I had something similar locally.

There is one last breaking change I found in the keymap deserializer. The new toml version can only serialize to String not to &str`. You can include the following commit to fix that:

5d0d6df

Once that is done this can be merged I believe

The new version of the `toml` crate is based on `toml_edit` and does
not support zero copy deserialization anymore. So we need to deserialize
`String` instead of `&str` in the keympa
@the-mikedavis
Copy link
Member

the-mikedavis commented Jan 24, 2023

For #4241 do you mean it's fixed by the error message improvements? I see

Bad config: TOML parse error at line 2, column 1
  |
2 | é = "collapse_selection"
  | ^
invalid key

Press <ENTER> to continue with default config

which is a nice improvement

@pascalkuthe
Copy link
Member

For #4241 do you mean it's fixed by the error message improvements? I see

Bad config: TOML parse error at line 2, column 1
  |
2 | é = "collapse_selection"
  | ^
invalid key

Press <ENTER> to continue with default config

which is a nice improvement

toml is correct here. é is invalid according to the toml spec. You need to quote it so "é". It sounded like from the issue that ö and é didn't work but ö works for me now. But I think I was wrong about this actually because it also works on master. I think I just missunderstood one comment there.

So never mind about that, sorry I misremembered the issue.

That issue can be close regardless d I think. The error message is pretty good now tough so I stil think that issue can be closed. I will add something FAQ.

@the-mikedavis the-mikedavis merged commit e83ce72 into master Jan 24, 2023
@the-mikedavis the-mikedavis deleted the dependabot/cargo/toml-0.6.0 branch January 24, 2023 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Area: Dependency rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants