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

diff: Provide an option to set the delimiter for the output #1396

Closed
2 tasks done
janriemer opened this issue Oct 31, 2023 · 5 comments · Fixed by #1402
Closed
2 tasks done

diff: Provide an option to set the delimiter for the output #1396

janriemer opened this issue Oct 31, 2023 · 5 comments · Fixed by #1402
Assignees

Comments

@janriemer
Copy link

janriemer commented Oct 31, 2023

Feature request

As a user of diff I'd like to be able to determine which delimiter (e.g. ',' or ';') is used for the output/result, so that I can use the resulting CSV with this custom delimiter. Currently, the output/result always has the default delimiter (',').

Possible solution

A possible solution for this is to have an additional flag --delimiter-result, which works analog to --delimiter-left and --delimiter-right respectively, but determines the delimiter that is used for the diff result.

Possible alternatives

An alternative to the above solution could be to use the same delimiter for the result that is used for the input CSVs (--delimiter-left and --delimiter-right).
The problem with this solution is: how to decide, which delimiter to use for the result, when --delimiter-left and --delimiter-right are different?

Additional context

This feature request aligns well with #1326, because that issue has been the first, where additional flags/options for the diff result had to be considered.

@jqnatividad What do you think about this? 🙂 You can assign me for this.

Tasks

  • Rename current flag --no-headers-result -> --no-headers-output (see discussion)
  • Implement flag --delimiter-output

=> All tasks complete! See PR #1402

@jqnatividad
Copy link
Collaborator

Hi @janriemer , your proposed solutions sounds good to me.

diff is your baby I'll defer to your choices, but do consider using --delimiter-output instead for consistency with the --output option.

@janriemer
Copy link
Author

@jqnatividad Yes, good point regarding consistency. 👍 I've actually deliberately chosen --delimiter-result instead of delimiter-output, due to the just merged PR #1395 (thank you!), where the option is --no-headers-result as well.

Regarding --output option itself: I see this option more like a direction, where the result should go (in this case to a file, instead of stdout).

So I propose, we use the term result instead of output from now on for the result of the diff.

What do you think?

Naming things is hard 😩

@jqnatividad
Copy link
Collaborator

OK @janriemer... though --output is the standard across the qsv suite and can be argued to indicate direction as well, I'll defer to your preference so long as its consistent within the diff command.

@janriemer
Copy link
Author

Hi @jqnatividad

thank you for the pointer! 👍 I wasn't aware of qsv using output terminology across different commands.

To be consistent, I propose to use output for diff then, too. This requires a change in the current flag --no-headers-result -> --no-headers-output, though (I've noted this as a task within this issue).

I hope, you haven't released this new feature of diff yet, so we won't need a breaking change. 🤞
In case you haven't: please hold the release regarding diff for now until we have reached consistency for diff with these new [...]-output flags (I hope this is possible for you without blocking other releases!?).

Thank you! 🙂

@jqnatividad
Copy link
Collaborator

Great!
And there won't be a release anytime soon. Most likely, the next one is in December as we're cooking up some new things that we'll unveil during the next release. 😉

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 a pull request may close this issue.

2 participants