asir66 commented on PR #263:
URL: 
https://github.com/apache/teaclave-trustzone-sdk/pull/263#issuecomment-3729034304

   After reviewing a more canonical Cargo subcommand implementation, I noticed 
that cargo-fmt handles the Cargo-injected subcommand argument in a different 
and more robust way.
   
   In cargo-fmt, the implementation explicitly drops the first occurrence of 
the subcommand name (fmt) at program entry, rather than relying on a positional 
placeholder in the argument parser. This behavior is intentional and aligns 
well with Cargo’s external subcommand invocation model.
   
   When [Cargo invokes an external 
subcommand](https://doc.rust-lang.org/cargo/reference/external-tools.html?utm_source=chatgpt.com#custom-subcommands),
 it follows the convention:
   ```
   cargo <subcommand> <args...>
   → cargo-<subcommand> <subcommand> <args...>
   ```
   
   As a result, a well-behaved Cargo subcommand should ideally support all of 
the following invocation forms:
   
   - cargo <subcommand> <args...>
   - cargo-<subcommand> <args...>
   - cargo-<subcommand> <subcommand> <args...>
   
   I verified that cargo-fmt conforms to this convention, while cargo-sgx 
currently does not handle all of these cases consistently.
   
   For reference, cargo-fmt implements this logic by filtering out the first 
occurrence of the injected subcommand name at the very beginning of execution:
   ```rust
   fn execute() -> i32 {
       // Drop extra `fmt` argument provided by `cargo`.
       let mut found_fmt = false;
       let args = env::args().filter(|x| {
           if found_fmt {
               true
           } else {
               found_fmt = x == "fmt";
               x != "fmt"
           }
       });
   
       let opts = Opts::parse_from(args);
   }
   ```
   
   Following the same rationale, I propose adopting an equivalent approach for 
cargo-optee, by removing the first occurrence of "optee" from the argument list 
at program entry:
   ```rust
   // Drop extra `optee` argument provided by `cargo`.
   let mut found_optee = false;
   let filtered_args: Vec<String> = env::args()
       .filter(|x| {
           if found_optee {
               true
           } else {
               found_optee = x == "optee";
               x != "optee"
           }
       })
       .collect();
   
   let cli = Cli::parse_from(filtered_args);
   ```
   
   This approach avoids introducing Cargo-specific positional arguments into 
the CLI definition, keeps the argument parsing logic explicit and localized, 
and ensures consistent behavior across all supported invocation forms.
   
   I believe this pattern is both more idiomatic and better aligned with 
existing Cargo tooling practices, as demonstrated by cargo-fmt.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to