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

All subcommands can use the flag package to describe their options #50

Closed
sirnewton01 opened this issue Jul 16, 2018 · 8 comments
Closed
Assignees

Comments

@sirnewton01
Copy link
Collaborator

sirnewton01 commented Jul 16, 2018

It turns out that with a little customization the flag package can support multiple instances of the same flag, aliased flags and flags that detect when they are not specified. This is probably sufficient to convert all of the subcommands over and get the benefit of the scaffolding in place to display the global and local options.

The key is the flag.Value and flag.Getter interfaces in conjunction with the flag.Var() function. This allows one to define their own value type and capture the Get() and Set() calls. We could introduce some optional, alias and multi value string values in dgit and use those throughout.

For the non-flag portions of the subcommands (e.g. "...", "") there is a way to add those details to the usage from the main.go. See #49 for examples.

@driusan
Copy link
Owner

driusan commented Jul 17, 2018

We already have hacks for aliases by setting multiple flags, and then or'ing them to get the result (or checking if either are non-default values for int/strings/etc), but if there's a more robust way to do it (or one that groups them together in output), we should do it.

But even with all that, we're still missing order sensitivity for a few commands.

@sirnewton01
Copy link
Collaborator Author

I've added an example for two aliased flags in #48. When converting other aliased flags it should be possible to extract a more abstract structure and functions to make reuse easier.

@driusan do you have an example in the code of order sensitivity for flags? I would like to have a look at these cases so that I can consider them in the design.

@driusan
Copy link
Owner

driusan commented Jul 17, 2018

I don't know if there's any we currently support, but rev-list has a --not option that inverts the meaning of commits until the next --not

@sirnewton01
Copy link
Collaborator Author

I have open PR #52 with proposed changes for this.

@sirnewton01 sirnewton01 self-assigned this Jul 18, 2018
@driusan
Copy link
Owner

driusan commented Jul 18, 2018

If we just want to put the infrastructure in place so that we can run t0000-basic.sh once we've fixed those four, for now we could just run one of the test cases that spuriously report passing for dgit like t0006-date.sh.

(We could also consider conditionally running the whole suite based on an environment variable, and including that version in an allowed_failure build.)

@driusan
Copy link
Owner

driusan commented Jul 18, 2018

I suspect someone could also fully implement fix t0007-git-var.sh pretty quickly. There's only 7 test cases, and most of them just fail because the git var command isn't implemented at all (but I think the functionality of git var is a subset of git config, which is implemented..)

@sirnewton01
Copy link
Collaborator Author

I have raised PR #58 to add the git var command and the official test suite.

@driusan driusan closed this as completed Jul 20, 2018
@driusan
Copy link
Owner

driusan commented Aug 2, 2018

update-index is another case where the flags are context sensitive. For instance, the setup of t1001-read-tree-m-2way.sh calls git update-index --add frotz bozbar --force-remove rezrov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants