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.

Sign up for GitHub

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

Jump to bottom

cmd/cue: remove support for the deprecated short form of "cue cmd" in v0.11 #2519

Closed
mvdan opened this issue Aug 2, 2023 · 3 comments
Closed

cmd/cue: remove support for the deprecated short form of "cue cmd" in v0.11 #2519

mvdan opened this issue Aug 2, 2023 · 3 comments
Assignees
@mvdan
Labels
NeedsDecision

Comments

@mvdan
Copy link
Member

mvdan commented Aug 2, 2023

We already have #1325 as a rethink and redesign for cue cmd. Before that happens, and while we still support the existing cue cmd, I think we should deprecate and later remove support for the "short form" or shortcut that is using cue foo instead of cue cmd foo.

In #1325 (comment) I raised some arguments in favor of this, and I believe others like @jpluscplusm and @myitcv share similar feelings.

The implementation of cmd/cue is also made significantly more complex due to this; for example, see https://review.gerrithub.io/c/cue-lang/cue/+/557127. It's not just that there's more code to maintain, but it's very tricky to properly implement these cue cmd shortcuts as they are effectively a fallback sub-command which requires loading a CUE package first, to discover which commands exist in *_tool.cue files.

The added complexity in cmd/cue also caused a number of bugs in the past:

Intuitively, it's also somewhat confusing that cue -t foo=bar foo works (injecting a tag to a command), but cue -t foo=bar fmt does not work, since -t is not actually a global flag even though it looks like one.

I propose that we start showing a deprecation warning when someone calls cue foo in v0.7.0, asking them to use cue cmd foo instead, and in v0.8.0 we can entirely remove support for the short form. With v0.7.0, we should also update our documentation and guides so that we don't encourage the use of the short form anywhere.

For example, in v0.7.0:

$ cue -t foo=bar foo
warning: the "cue cmd" abbreviation is deprecated in v0.7, and will be removed in v0.8. instead, use:
    cue cmd -t foo=bar foo
cueckoo pushed a commit that referenced this issue Aug 2, 2023
@mvdan
cmd/cue: fix regression when injecting vars with cue cmd
da3ee7d 
The -t and -T flags can be used like:

    cue cmd -t foo=var -T mycmd .

However, we also support a short form, where "cmd" can be omitted:

    cue -t foo=var -T mycmd .

After the refactor in https://cuelang.org/cl/552733,
we made cmd/cue more principled in how it handles the root command,
and we broke those -t and -T flags in the "cue cmd" short form.
We didn't intend to - they just started being treated as global flags.

A simple and easy fix is to actually declare these flags globally,
and not just as part of the "cue cmd" sub-command.
Since each of these flags now exists in two levels,
the only place where they are consumed, the setTags function,
now checks both flag sets and joins their values.

Note that we make the new global flags hidden,
so that "cue help" doesn't start showing them as global flags.
We don't really intend them as global flags for every command.
We also avoid adding them to PersistentFlags,
so that they are not inherited by sub-commands like fmt.

This fix is preferrable to an entire revert of the previous CL,
as otherwise commands like "cue help" would become very slow again.

I've also filed #2519 to propose deprecating the "cue cmd" short form.

Fixes #2510.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I254e1c4e8ef4d6ffa7d6142c75abee5f1a37a810
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/557127
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
@jpluscplusm
Copy link
Collaborator

I support the short, non-cmd invocation's removal.

Separately: whilst CUE doesn't have 30 years of precedence-creating history to think about, it might be worth keeping in mind the kerfuffle that adding obsolescence/deprecation warnings to the stderr of egrep (and siblings) created relatively recently: https://unix.stackexchange.com/a/727773.

Is a cue_tool that prints to stderr only in exceptional cases a use case that's currently possible?

@mvdan mvdan mentioned this issue Aug 14, 2023
Closed
@mvdan mvdan changed the title cmd/cue: deprecate the short form of "cue cmd" in v0.7, and remove its support in v0.8 cmd/cue: deprecate the short form of "cue cmd" in v0.8, and remove its support in a later release Mar 21, 2024
@mvdan mvdan changed the title cmd/cue: deprecate the short form of "cue cmd" in v0.8, and remove its support in a later release cmd/cue: deprecate the short form of "cue cmd" in v0.9, and remove its support in a later release Mar 21, 2024
@mvdan
Copy link
Member Author

mvdan commented Mar 21, 2024

The deprecation didn't happen for v0.7, but starting it now for v0.9.

cueckoo pushed a commit that referenced this issue Mar 21, 2024
@mvdan
cmd/cue: deprecate cue somecmd in favor of cue cmd somecmd
992f559 
v0.9.0-alpha.1 will be released with the deprecation warning to stderr,
and a future release like v0.10 or v0.11 can remove support entirely.

Note that even when we remove support for the short form,
this shouldn't be a hard breakage for any users, as they can switch
to the long form by adding the "cmd" argument.

Also rewrite a few tests which didn't need to use the short form.
Only one other test case remains with the short form,
as it explicitly tests for an edge case bug relating to the short form.
That test case can be removed entirely once we drop support.

Updates #2519.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I04aa48b048bb3f1c397f8b95a36933d797734930
cueckoo pushed a commit that referenced this issue Mar 21, 2024
@mvdan
cmd/cue: deprecate cue somecmd in favor of cue cmd somecmd
e8ecf23 
v0.9.0-alpha.1 will be released with the deprecation warning to stderr,
and a future release like v0.10 or v0.11 can remove support entirely.

Note that even when we remove support for the short form,
this shouldn't be a hard breakage for any users, as they can switch
to the long form by adding the "cmd" argument.

Also rewrite a few tests which didn't need to use the short form.
Only one other test case remains with the short form,
as it explicitly tests for an edge case bug relating to the short form.
That test case can be removed entirely once we drop support.

Updates #2519.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I04aa48b048bb3f1c397f8b95a36933d797734930
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1185473
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.io>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
@mvdan mvdan self-assigned this Jun 7, 2024
@mvdan
Copy link
Member Author

mvdan commented Jun 7, 2024

Aiming to remove support for the deprecated form in the first v0.11 alpha; the v0.9 and v0.10 release cycles will be short, around three months, so deprecation for one release cycle is a bit tight.

@mvdan mvdan changed the title cmd/cue: deprecate the short form of "cue cmd" in v0.9, and remove its support in a later release cmd/cue: remove support for the deprecated short form of "cue cmd" in v0.11 Jun 7, 2024
cueckoo pushed a commit that referenced this issue Aug 19, 2024
@mvdan
cmd/cue: refactor help texts and topics
7e589e3 
Because additional help topics used to be non-runnable commands hanging
from the root `cue` command, they used to behave in rather odd ways.
For example, even though we often suggested using them via
`cue help filetypes`, they still worked as `cue filetypes` or
`cue filetypes --help` for no good reason.

Hang the non-runnable commands from the `cue help` subcommand,
meaning that `cue filetypes` is now treated as an unknown command.
This seems better for the user experience too; for example,
if a user tried to tab-complete `cue fi`, we should only suggest
`cue fix` and not `cue filetypes`.

Because help topics now hang as subcommands from `cue help`,
spf13/cobra's default help template would no longer show them
in the help text for the root command.
Start using our own template, as a simplified and tweaked version of
the default template, which resolves this issue.

Our own template also tweaks a few other sharp edges while here:
* Do not show global flags in `cue help`; they take up space,
  and they aren't useful unless a user is running a subcommand.
  Subcommand help texts already list the same global flags.
* Suggest `cue help [command]` rather than `cue [command] --help`,
  which reads better and is shorter to type, and is also consistent
  with additional help topics such as `cue help filetypes`.
* Hide `cue help` from the available commands list;
  we already suggest it as the way to get help texts, so we don't need
  to show it twice in the top-level help text.

This is a precursor to removing support for `cue foo` as a short-hand
for `cue cmd foo`, as some of the logic to handle these extra
root-level commands is shared.

For #2519.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I76d0c687e88bc3f237d952a98b4578516f36c5c0
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199622
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Matthew Sackman <matthew@cue.works>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mvdan mvdan

Labels
NeedsDecision
Projects
Release v0.11
Status: v0.11.0-alpha.1
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jpluscplusm @mvdan