Skip to content

bugfix: add "to yml" command #15254

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

Merged
merged 2 commits into from
Mar 6, 2025
Merged

Conversation

LoicRiegel
Copy link
Contributor

Description

This fixes #15240, which can be closed after merge.

User-Facing Changes

  • user get now use to yml -> exactly the same as to yaml

2025-03-06_00h01_27

Tests + Formatting

Cargo fmt and clippy 🆗
I added a test in the only place I could find where to yaml was already tested.

I didn't see the save.rs::convert_to_extension function tested anywhere, but maybe I missed it.

After Submitting

Not sure this needs an update on the documentation ❓ What do you suggest?

Copy link
Member

@sholderbach sholderbach 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 was thinking if we could get by with a .nu version of this as part of the std prelude. But supporting all the flags and documentation like this may still be kind of messy. So I am fine with the Rust-duplication alias.

Co-authored-by: Stefan Holderbach <[email protected]>
@LoicRiegel
Copy link
Contributor Author

Thanks!

I was thinking if we could get by with a .nu version of this as part of the std prelude. But supporting all the flags and documentation like this may still be kind of messy. So I am fine with the Rust-duplication alias.

I thought the duplication was okay because that's already the case for FromYaml and FromYml, but if there's a better approach I'm okay to change

@LoicRiegel
Copy link
Contributor Author

Not sure this needs an update on the documentation ❓ What do you suggest?

Also what's your take on this?

@sholderbach
Copy link
Member

I thought the duplication was okay because that's already the case for FromYaml and FromYml, but if there's a better approach I'm okay to change

Yeah fine based on precedent as well. Let's go with this approach for now

Not sure this needs an update on the documentation ❓ What do you suggest?

Also what's your take on this?

I think it suffices that the command will appear in the autogenerated command help.
We may want to improve the docs around this open/save mapping to to ... and from ... at some point.
But for this change I think you don't need to change anything in the permanent docs. Just free free to add it to the release notes as a new command with a brief example that saving with .yml ending works now.

@sholderbach sholderbach added file-format Parsing/Writing of file formats/protocols new-command pr:release-note-mention Addition/Improvement to be mentioned in the release notes labels Mar 6, 2025
@sholderbach sholderbach merged commit 0e6e9ab into nushell:main Mar 6, 2025
15 checks passed
@github-actions github-actions bot added this to the v0.103.0 milestone Mar 6, 2025
@LoicRiegel
Copy link
Contributor Author

Just free free to add it to the release notes as a new command with a brief example that saving with .yml ending works now.

Sure! How does it work, do I just push a new commit on the release-notes-0.103.0 branch?

@sholderbach
Copy link
Member

If you edit it in the web interface it will probably ask you to open a PR. If you start your changes locally, you will have to set the target branch of the PR to release-notes-0.103.0.

@LoicRiegel
Copy link
Contributor Author

Hi @sholderbach I did that PR for the release note but I think nobody saw it
I saw the contribution mentioned in the weekly blog, though

@sholderbach
Copy link
Member

Thanks! Sorry that I didn't get around to it. Nice user-friendly writeup!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
file-format Parsing/Writing of file formats/protocols new-command pr:release-note-mention Addition/Improvement to be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saving a data structure as yaml works with .yaml extension but not .yml
2 participants