Let toml set edit the actual file

[Busyvar]

Hi,

As many other tools like sed , i think it would be interesting to confirm file editing with a flag instead of using stdout.
So the argument after 'set' won't be repeated in the command.

What do you think?

Regards,
Busyvar

[kurtbuilds]

FYI I added this in my fork:

https://github.com/kurtbuilds/toml-cli

You have to git clone it, then cargo install --path ., since the fork isn't on crates.io.

Since this project appears to be unmaintained, I reached out to Greg about contributing to this repo & crates.io to keep this tool maintained.

[Busyvar]

Thank you @kurtbuilds , i saw you made a PR with it,
#8

Hope you will be soon accepted as a maintainer for this project.

[gnprice]

Hello! Yes, I definitely agree this is an important feature.

One question this issue thread is a good place to discuss is what exactly the interface should look like. I'd be glad to hear feedback on that from anyone who's used this toml command.

From discussion at #13 (comment) on @liubin's PR #13, here's one thought: instead of just making this an option, how about we go further and make it the default behavior of toml set?

I feel like for most use cases, you're always going to want to write the edited file back. So it'd be cleaner if you could say simply toml set foo.toml …, rather than saying toml set --in-place (or whatever we might call this option) all the time.

That would also make it align better with the name "set" than it currently does.

We can always include an option to go back to the other behavior, with a name like --dry-run or --print.

[gnprice]

Now, making this happen by default would be an incompatible change. But as the README says:

The command's status is experimental. The current interface does not yet serve its purposes as well as it could, and incompatible changes are anticipated.

Making toml set update the actual file on disk is exactly the sort of change I had in mind with that warning.

However, I'd certainly be interested in hearing feedback on that incompatible change: does anyone have the existing toml set running in scripts, in a way where they'd be concerned that a sudden change to operating in place would break something?

If so, there's also an intermediate option to take a more sequenced approach, something like:

• First, add --in-place (or --overwrite, or --write, or whatever name), and at the same time also --dry-run which explicitly gets the current behavior.
• When neither of those options is passed, preserve the current --dry-run behavior -- but print a warning that says this is deprecated, and the default may change.
• After that's out in a release, perhaps take another step by making it an error to run toml set with neither --in-place nor --dry-run.
• Some time after either or both of those is out in a release (perhaps a couple of months? shorter? longer?), take the final step by making toml set have the edit-in-place behavior by default.

That way we end up with the same behavior as discussed in the previous comment #7 (comment) , where toml set does indeed set the value in the given file, but we also have an intermediate period of warnings and/or errors to help people spot any call sites that need to be updated.

[gnprice]

OK, I'm going to go with some version of the "more sequenced approach" described in my previous comment #7 (comment). The endpoint will be that a plain toml set actually updates the file.

I plan to call the options --write (for the good behavior, the future default) and --print (for the current behavior.)

The warning/error messages that transitional versions print on a plain toml set (telling you to add an explicit --print or --write) will be loud — designed to be easy to spot in a long build log.

---

One thing that informs this decision is that I went and did a bit of a survey of uses of toml-cli that I can readily find using GitHub search. Two observations are relevant to this thread:

• Every single use of toml set that I found really wants the behavior of editing the actual file. They redirect to a temporary file and mv that over the original, or slurp the output to a shell variable and echo that to the original file, or another similar solution.

  This observation reinforces my view that the right interface is to have plain toml set overwrite the file, and let any other behavior be an option.

• Very few users apply any kind of version pinning; instead it's simply cargo install toml-cli.

  Frankly, writing an unversioned cargo install foo in a script seems unwise in general. (Much safer would be to use a caret constraint like cargo install foo@^1.2.3, in order to at least avoid getting versions the maintainer has explicitly marked as incompatible with the one you're testing on.) Especially so with a package whose README explicitly forecasts incompatible changes, as toml-cli's does.

  But that's what people have done. So that informs what kind of transition steps will and won't be effective in helping people make a smooth upgrade with a minimum of fuss.