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

Upgrade to clap v4 #724

Closed
wants to merge 6 commits into from
Closed

Upgrade to clap v4 #724

wants to merge 6 commits into from

Conversation

nezuo
Copy link
Contributor

@nezuo nezuo commented Jul 9, 2023

I need clap v4 for my planned changes for #715 because it has a new feature called argument relations. I can use the feature to stop users from using project and output flags at the same time.

New output

Clap changes the output from v3 to v4. Here's what the difference looks like:

Clap v3

image

Clap v4

image

Notice the lack of colors, the new casing, the bold, and the underlines.

Changes

  • I no longer display the author since rojo no longer has a sole author.
  • For whatever reason, clap v4 does not allow a default of "" for paths. Instead, I use the actual paths like default.project.json. Now it shows up in the help output:
    image

src/cli/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@Dekkonot Dekkonot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I don't care about the formatting changes and the loss of colors is... tragic, but acceptable in my opinion. Hesitantly, I think this is fine with me.

What does the alternative for not upgrading look like? I know it wouldn't display as exclusive in help but is that necessarily a deal breaker? We could always just raise an error if both fields are filled in.

@nezuo
Copy link
Contributor Author

nezuo commented Jul 12, 2023

Here's a few screenshots of what the output looks like with argument relations for reference:
image
image
image

@Dekkonot
Copy link
Member

That's kind of a funky syntax. I know it's not something you can control, but I don't like it. That said, it's not a deal breaker because most people will probably just be memorizing how to use a command anyway.

Would like to hear @Kampfkarren's and @kennethloeffler's inputs on this too, since it's a bit more of a design choice than normal and I want to make sure that multiple eyes okay it.

@kennethloeffler
Copy link
Member

Notice the lack of colors

taking away my colors? Everything is already gray enough 😭

Jokes aside... I agree with dekkonot, the changes to formatting/colors aren't a big problem, but it may be a smarter engineering decision to hold off on upgrading until we have a more compelling reason to do it (like maybe another mutually exclusive set of flags, or anywhere else we could use other new clap features)

@nezuo
Copy link
Contributor Author

nezuo commented Jul 14, 2023

I've decided to close this PR and implement the plugin flag without clap v4: #735.

The removal of colors is a somewhat controversial change, and although the output in #735 isn't amazing, clap v4's output isn't either.

If clap ever decides to support colors again, it's easy to upgrade.

@nezuo nezuo closed this Jul 14, 2023
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.

3 participants