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

New xml format #7947

Merged
merged 17 commits into from
Mar 11, 2023
Merged

New xml format #7947

merged 17 commits into from
Mar 11, 2023

Conversation

NotLebedev
Copy link
Contributor

@NotLebedev NotLebedev commented Feb 2, 2023

Description

Changes old from xml to xml data formats. See #7682 for reasoning behind this change.
Output is now a series of records with tag, attributes and content fields.

Old:
image
New:
image

User-Facing Changes

New output/input format, better error handling for from xml and to xml commands.

Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace to check that all tests pass

After Submitting

If your PR had any user-facing changes, update the documentation after the PR is merged, if necessary. This will help us keep the docs up to date.

@rgwood
Copy link
Contributor

rgwood commented Feb 7, 2023

I'm excited for this PR. XML has been a weak spot for a long time.

@codecov
Copy link

codecov bot commented Mar 11, 2023

Codecov Report

Merging #7947 (383c0bd) into main (e435196) will increase coverage by 0.30%.
The diff coverage is 60.54%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7947      +/-   ##
==========================================
+ Coverage   68.25%   68.55%   +0.30%     
==========================================
  Files         620      620              
  Lines       99355    99600     +245     
==========================================
+ Hits        67813    68284     +471     
+ Misses      31542    31316     -226     
Impacted Files Coverage Δ
crates/nu-command/src/formats/to/xml.rs 63.93% <54.68%> (-20.02%) ⬇️
crates/nu-command/src/formats/from/xml.rs 76.48% <66.28%> (-16.42%) ⬇️

... and 6 files with indirect coverage changes

@NotLebedev NotLebedev marked this pull request as ready for review March 11, 2023 19:50
@fdncred
Copy link
Collaborator

fdncred commented Mar 11, 2023

I'd still vote to re-run the CI and land this when it's green. It's better than what we have now.

@sophiajt
Copy link
Contributor

Seconding @fdncred. I vote we should land this, too.

@sophiajt sophiajt merged commit a13946e into nushell:main Mar 11, 2023
@NotLebedev NotLebedev deleted the new-xml-format branch March 12, 2023 08:28
@rgwood
Copy link
Contributor

rgwood commented Mar 13, 2023

This should get mentioned as a breaking change/new feature in the 0.77 blog post. @NotLebedev do you have time to write that up today or would you like someone else to? nushell/nushell.github.io#800

@NotLebedev
Copy link
Contributor Author

This should get mentioned as a breaking change/new feature in the 0.77 blog post. @NotLebedev do you have time to write that up today or would you like someone else to? nushell/nushell.github.io#800

Yes I have time. Will do shortly.

@NotLebedev NotLebedev mentioned this pull request Mar 13, 2023
fdncred pushed a commit that referenced this pull request Apr 5, 2023
# Description

Add `xaccess`,`xupdate` and `xinsert` scripts to standard library. They
allow accessing and manipulating data in new xml format
#7947 with relative ease.

Access some data in nushell xml structure:

![image](https://user-images.githubusercontent.com/17511668/224785447-317359e2-1430-4dfc-9307-73f1d5e50098.png)

Update attributes of xml tags matching a path:

![image](https://user-images.githubusercontent.com/17511668/224785506-85e9aa30-b36b-43db-af1d-2f4460563124.png)


# User-Facing Changes

New commands `std xaccess`, `std xupdate` and `std xinsert`

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
39555 added a commit to 39555/nu_scripts that referenced this pull request May 12, 2024
`from xml` in modern `nu` has different shape.
Output is now a series of records with tag, attributes and content fields

nushell/nushell#7947
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

Successfully merging this pull request may close these issues.

None yet

4 participants