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

choice entry does not include prompt #63

Open
Vollbrecht opened this issue Feb 5, 2025 · 6 comments
Open

choice entry does not include prompt #63

Vollbrecht opened this issue Feb 5, 2025 · 6 comments

Comments

@Vollbrecht
Copy link

In my Kconfig file the choice section also can have a optional promt (aka a name for the choice span) if i understand correctly. At least its mention in the official docs.

I also have a Kconfig file that seams to utilize it ;D

@Mcdostone
Copy link
Owner

Mcdostone commented Feb 5, 2025

Hi,

do you have any example?
I'ts possible I have forgotten to implement the parsing for the choice section

Would you like to implement it :) ?

@Vollbrecht
Copy link
Author

Vollbrecht commented Feb 5, 2025

I currently try to find an example in the kernel source for that but a minimal search didn't find a inlined prompt here hmm....

Truth to be told I actually am not using the library to parse the linux kernels Kconfig files, rather using it on another project, that happens to also use Kconfig files. Though it seams they are using a slight variation of the standard 😬 Its for parsing the esp-idf sdk ( espressif esp32 micro controller). They wrote there own specification up here.

But my plan was not to barge in on your project with non compliant stuff. I plan to fully integrate it so i get it to working. I will try to make it fully compatible to the linux kernal Kconfig, and would PR all stuff. I would understand if you don't want to have it in your branch though.

Depending on if i can integrate it without making it incompatible to the kernel's Kconfig definition, i would do so directly, or if that is not possible ( or have other negative effects when not used) create a cargo feature gate like esp-idf to make it compatible on demand.

@Mcdostone
Copy link
Owner

Oh I see. I tested nom-kconfig accross most of the linux kernel Kconfig files so I would be surprised If a missed a case.
A cargo feature could be a good idea indeed. Let me know if I can help you somehow!

@Vollbrecht
Copy link
Author

Vollbrecht commented Feb 6, 2025

A note on the choice topic itself. In the choice struct the prompt would be currently captured as a "option" in the Vec entry, so something like this in the kernel should be captured.

Its handled a little different in the case of other menu items like MainMenu and Menu were the prompt is a separate part of parent struct. So maybe it would also be nice to put the prompt out in the Choices case.

Background:
The standard itself defines strictly here that -> "Every menu entry can have at most one prompt, which is used to display to the user." and also that the absence of a prompt is associated with being a "non-visible symbol".

A menu entry start definitions is either one of "config", "menuconfig", "choice", "comment", "menu" according to this

I think with that above knowledge it would make sense to make sure that:

  • evey menu entry has a directly associated single prompt entry
  • the absence of it is special as it indicates non-visibility to the user

That would lead me to a struct field like { .prompt: Option<String> } what do you think about that?


A separate 2nd point i want to talk about is: Can a choice have a symbol associated? If we look at the current source of this library the comment suggest that something like this exist. But i cannot find that this is defined in the official docs? The funny thing is currently my biggest problem is that my Kconfig file defines symbols on choices and they are currently not parsed correctly :D I am talking about this comment. It would imply that that would be "OK".

@Mcdostone
Copy link
Owner

Regarding the Choice entry, it sounds like a good idea. I didn't notice this rule when I wrote the parser.

Something like this would be better maybe ?

pub struct Choice {
    pub options: Vec<Attribute>,
    pub entries: Vec<Entry>,
    pub prompt: Option<String>,
}

I think you're right on this point.

@Mcdostone
Copy link
Owner

Mcdostone commented Feb 6, 2025

Regarding your second point the comment you are refering to was present in the official Kconfig documentation at the time I wrote this parser (thanks git):
https://github.com/torvalds/linux/blob/28d49e171676afb7df7f47798391364af9abed7f/Documentation/kbuild/kconfig-language.rst

It looks like they changed the documentation in 2024:
https://lore.kernel.org/lkml/[email protected]/

the optional attribute is unused now. So maintainers removed it from the documentation and the C parser.

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

No branches or pull requests

2 participants