-
-
Notifications
You must be signed in to change notification settings - Fork 51
Description
Summary
Following the changes in 938f24c, to require an Issue to be opened prior to a PR I'm opening one.
I believe the generation of shell completions (via clap_complete, clap_complete_nushell, and potentially others) and environment hook files can be shifted from runtime invocation to static compile-time evaluation for improved efficiency and cleanliness.
Projects like Yazi demonstrate a cleaner approach by handling this at compile time. My understanding is that we can use the included generate_to function from clap_complete (docs here) to produce the completion files during the build process via a build.rs script. These files can then be embedded into the binary using include_str! (e.g., by generating them to OUT_DIR and including via concat!(env!("OUT_DIR"), "/completion.file")). At runtime, when the user requests output via commands like bob --complete fish, we simply print the embedded string content instead of generating on-the-fly.
clap_complete_nushell also provides a Nushell generator that implements the Generator trait, so it can be used with generate_to in the same way for compile-time generation. This approach can extend to other shells or hooks as needed.
I strongly believe that non-essential runtime computations, like completion generation, should be moved to compile time where possible to reduce overhead and ensure consistency.
I'm proposing to add a build.rs script to generate and embed these completions and hooks, updating the relevant command handlers to output the pre-generated content.
There are many benefits to this, among them:
- Improved Runtime Performance: Eliminates the need to dynamically generate completions on every invocation, reducing latency and CPU usage for users requesting them.
- Consistency and Reliability: Pre-generated files ensure completions stay in sync with the CLI structure without runtime desync risks; embedding avoids file distribution issues.
- Cleaner Codebase: Removes runtime generation logic, replacing it with simple string outputs; aligns with idiomatic Rust patterns for build-time artifacts.
- Reduced Dependencies at Runtime: No need to pull in generation crates post-build, potentially slimming the binary.
- Better User Experience: Faster response to completion requests; easier packaging if hooks/completions are embedded.
- Maintainability Improvements: Centralizes generation in
build.rs, making it easier to add support for new shells without runtime branching.
Justification for Change
The current implementation relies on runtime generation (and also overshadows the existing implementation of clap_complete's Shells enum instead of extending it via 'NewType' patterns), which adds unnecessary overhead for a static output that doesn't change per invocation. This can be particularly noticeable on resource-constrained systems or when frequently requesting completions. By contrast, compile-time embedding (as enabled by generate_to and include_str!) precomputes everything, turning the op into just a string print. While generate_to writes files to disk ('OUT_DIR') during build, combining it with include_str! allows embedding into the binary. This same pattern can also be extended to the environment hooks generation, albeit via manual content generation at compile time.
This pattern is common in Rust projects for static assets and avoids the pitfalls of manual static insertion (e.g., desync), as the files are regenerated on every build from the same Command definition.
Open to discussing implementation details on various aspects of course, such as handling multiple shells, environment hooks specifically, or any potential edge cases with embedding large strings.