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

act as textconv so can git diff inventory #295

Open
msftcangoblowm opened this issue Aug 19, 2024 · 4 comments
Open

act as textconv so can git diff inventory #295

msftcangoblowm opened this issue Aug 19, 2024 · 4 comments
Labels
area: cli 💻 issue: maybe 🤔 Being considered, but not certain type: enhancement ✨ Something to add

Comments

@msftcangoblowm
Copy link

Is your feature request related to a problem? Please describe.
git can be informed on which files are binary. And then instructed how to convert the binary files into text files, so git diff can work.

git passes in the .inv file name cuz the git diff is occurring on the .inv, not on the .txt file.

sphobjinv convert plain requires a hyphen to be passed in to have the output sent to stdout. Instead would like a option flag. So then sphobjinv can act as a textconv

docs/.gitattributes

[diff "inv"]
  textconv = sphobjinv --stdout convert plain 
  binary = true

Describe the solution you'd like
Provide a --stdout option flag as another means to specify the outfile hyphen.

Describe alternatives you've considered
Alternative approach, which i'm using now, is to mark the inventory files as binary and not be able to use git diff

docs/.gitattributes

*.inv binary

Additional context

#159 is a similar feature request, but for stdin
The difference is infile is a required positional argument whereas outfile is optional. So a --stdout flag would not change sphobjinv command line interface

perform diffs on binary files

the fallback, indicate files are binary

@msftcangoblowm
Copy link
Author

The textconv config option is used to define a program for performing such a conversion. The program should take a single argument, the name of a file to convert, and produce the resulting text on stdout.

2nd paragraph from perform diffs on binary files

Didn't see The program should take a single argument. Which would necessitate an additional entrypoint to act as a textconv program

sphobjinv-textconv infile

Which can only be used with inventory files and produces plain text onto stdout

@bskinn bskinn added type: enhancement ✨ Something to add issue: planned ⌚ Assigned to a specific version's milestone area: cli 💻 labels Aug 19, 2024
@bskinn
Copy link
Owner

bskinn commented Aug 19, 2024

I like this! And yeah, it should be a matter of writing a new ArgumentParser, wiring it into a new textconv function in e.g. sphobjinv.cli.core, and then adding a new entrypoint in pyproject.toml.

I probably won't be able to get to this right away -- if you're interested, feel free to put a PR together for it.

@msftcangoblowm
Copy link
Author

Thank you for your enthusiastic approval for this feature. That, and your advice, are very much appreciated and what i was seeking.

This is on me. I got this. ETA 2-3 days

msftcangoblowm added a commit to msftcangoblowm/sphobjinv that referenced this issue Aug 26, 2024
- feat: entrypoint sphobjinv-textconv ([bskinn#295])
- test: add sphobjinv-textconv unittests offline and online
- test: add integration test. Demonstrate git diff objects.inv
- test: add pytest module with class to interact with git
- docs: add step by step guide to configure git. Both bash and python code examples
- docs: added sphobjinv-textconv API docs
@msftcangoblowm msftcangoblowm mentioned this issue Aug 26, 2024
11 tasks
@bskinn bskinn added issue: maybe 🤔 Being considered, but not certain meta: yagni 🙅 Will only be considered if folks express interest and removed issue: planned ⌚ Assigned to a specific version's milestone meta: yagni 🙅 Will only be considered if folks express interest labels Nov 3, 2024
@bskinn
Copy link
Owner

bskinn commented Nov 3, 2024

Switching this from planned to maybe. See #296; I remain open to adding an implementation of the sphobjinv-textconv entrypoint as discussed there, though as discussed there I'm unsure whether it's necessary given that the current CLI can support git textconv functionality, albeit only with more-complex and less-readable syntax in the relevant git configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cli 💻 issue: maybe 🤔 Being considered, but not certain type: enhancement ✨ Something to add
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants