Mutable app functions are confusing to users and awkward to work with

[epage]

Please complete the following tasks

• I have searched the discussions
• I have searched the existing issues

Rust Version

rustc 1.55.0 (c8dfcfe04 2021-09-06)

Clap Version

master

Minimal reproducible code

let mut app = ...;
app.render_usage();

Steps to reproduce the bug with the above code

Program with the &mut API

Actual Behaviour

Its awkward and exposing of implementation details. I've seen user confusion akin to "Why does rendering need to modify the app?"

Other uses of this API, like clap_generate, need to use the hidden _build

Expected Behaviour

The types are clean and understandable for a user without ambiguous "modify after build"

Additional Context

I propose we add a struct AppParser that derefs to App.

• get_matches will consume App and use _build (only build what is needed for parsing)
• App::build(self) -> AppParser will call _build_all
  • Contains all of the existing &mut functions, but without the mut

We can implement the additions without breaking behavior; we just add deprecations. Then on the next breaking release, we remove the deprecated behavior.

Debug Output

No response

[pksunkara]

Same issue as print_*_help and write_*_help methods. I wanted them to be non-mut but couldn't find a way to do it without quickly blowing up the scope of it. IIRC there were some discussions around this when those signatures were implemented which might be useful here.

[epage]

What are your thoughts on the proposal for solving the mut problem?

IIRC there were some discussions around this when those signatures were implemented which might be useful here.

At least in my initial search, I found #540 which didn't have any useful conversation on the subject.

[pksunkara]

It would be a good approach to explore definitely. Will have to evaluate it properly when we are finalising the design though.

[pksunkara]

Heh, weird. Couldn't find the discussion now. Maybe it was on something else that was related? 🤷

[epage]

We should probably do #3089 first.
Done

[epage]

#950 needs us to rename the get_matches functions. We should coordinate these changes together to reduce user churn.

[epage]

A simple way we can handle this is if we add the type

pub struct Built<T>(T);

impl Built<Command> {
   pub fn get_arguments(&self) -> impl Iterator<Item=Built<Arg>>;

   pub fn get_subcommands(&self) -> impl Iterator<Item=Built<Command>>;
}

impl Built<&'_ Command> {
   pub fn get_arguments(&self) -> impl Iterator<Item=Built<Arg>>;

   pub fn get_subcommands(&self) -> impl Iterator<Item=Built<Command>>;
}

impl<T> AsRef<T> for Built<T>{
    #[inline]
    fn as_ref(&self) -> &T{
        &self.0
    }
}

impl<T> Borrow<T> for Built<T>{
    #[inline]
    fn borrow(&self) -> &T{
        &self.0
    }
}

impl<T> :Deref for Built<T> {
    type Target = T;

    #[inline]
    fn deref(&self) -> &T{
        &self.0
    }
}

We would also remove the get_arguments getters on Command in favor of get_arguments_mut to keep clear that Command isn't built

This still leaves trying to re-work the parser so it can work off of mut and immutable Command.