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

Introduce YAML support #67

Merged
merged 4 commits into from
Jun 20, 2023
Merged

Introduce YAML support #67

merged 4 commits into from
Jun 20, 2023

Conversation

corytodd
Copy link
Collaborator

@corytodd corytodd commented Jun 11, 2023

This introduces YAML support to the CLI. No changes to the library are intended. There are some minor changes to the CLI help menu but otherwise this is only a feature add.

  • CLI help now includes the default parameters when --help is specified
  • CLI --syntax is now limited to the list of built-in syntaxes (I always forget how to spell symmetric :) )
  • New YAML loader/dumper pair
  • Tests now explicitly cover all loader/dumper pairs

Fixes #66

@fzumstein
Copy link
Member

LGTM, the only thing I'd question is if you want to make pyyaml a hard dep or maybe just an optional one (pip install jsondiff[pyyaml])?

@corytodd
Copy link
Collaborator Author

I did consider that but thought I'd propose a hard dep first. Let me see if there is a clean-ish way to make the YAML features optional without breaking the AP and I'll get back to you.

@fzumstein
Copy link
Member

It's not a big deal, just thought I'd bring it up.

@fzumstein
Copy link
Member

There's now a conflict since I merged the rigthonly branch, do you mind fixing this then I can merge. Thanks!

Cory Todd added 4 commits June 20, 2023 11:35
Use ArgumentDefaultsHelpFormatter so we can see what the default
parameter values are.

Signed-off-by: Cory Todd <[email protected]>
Add help strings for parameters. Also restrict cli syntax options to
those already built in the library since a CLI user cannot provide their
own implementation.

Signed-off-by: Cory Todd <[email protected]>
Introduce a loader and dumper for YAML that can be used by the CLI. Add
tests for all loaders that exercise both forms (string and file-like).
Adds a YAML example to the README.

Signed-off-by: Cory Todd <[email protected]>
Include the new YAML tools in the __all__ export for consistency.

Signed-off-by: Cory Todd <[email protected]>
@corytodd
Copy link
Collaborator Author

done - I did look into making yaml optional and while doable, I think it will make things more confusing for the end user. For people installing from pip, the new dependency on pyyaml should be transparent, though I understand the desire to minimize dependencies.

@fzumstein fzumstein merged commit a5d4bbe into xlwings:master Jun 20, 2023
8 checks passed
@fzumstein
Copy link
Member

thanks!

@corytodd corytodd deleted the yaml-support branch June 20, 2023 22:10
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.

Introduce YAML support for cli
2 participants