Skip to content

Conversation

@ImplFerris
Copy link

espup allows to choose custom name for the toolchain other than "esp". It will be nice esp-generate also allow to specify custom toolchain name.

@bugadani
Copy link
Contributor

bugadani commented Nov 3, 2025

I agree we need this, and a CLI argument is a good enough starting point. Ideally, though, we should offer the option via the TUI. Since I think the PR already stands on its own, and asking you to come up with a completely new way to enter a string in esp-generate would not be reasonable, could you open an issue to track this feature, please?

Also, could you make the new flag optional, so that the tests can succeed?

@ImplFerris
Copy link
Author

ImplFerris commented Nov 3, 2025

asking you to come up with a completely new way to enter a string in esp-generate would not be reasonable, could you open an issue to track this feature, please?

I just noticed what you are referring to. I have been giving the project name and chip through the argument so far. so didnt notice the tui part for those inputs.

I believe it is small change to integrate into tui also. i can push that also in this PR

if args.toolchain_name.is_none() {
        let toolchain_name = Text::new("Enter Toolchain name:")
            .with_default("esp")
            .prompt()?;

        args.toolchain_name = Some(toolchain_name);
    }

@bugadani
Copy link
Contributor

bugadani commented Nov 3, 2025

That's not really what I'm looking for, though. Based on the chip, we know the target. We should check if there are multiple applicable toolchains installed, and list only those for the user, if there are more than one. We can even warn the user they have no applicable toolchain installed. Just inserting the name is somewhat convenient, but I believe esp-generate should hold the users' hands a bit better than that. And that doesn't sound like a 5-liner fix for this PR :)

@ImplFerris
Copy link
Author

Oh i was wondering why you asked to raise new issue for this simple task. Now i know 😀

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.

2 participants