nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

tools/kconfig2html.c: Add missing parse logic

Open patacongo opened this issue 3 years ago • 6 comments

Summary

I tried running tools/mkconfigars.sh for the first time in a long time and it failed due to some missing support in the parser: The following was supported

config FOO
  int "Enable foo"

But not this equivalent form:

config FOO
  int
  prompt "Enable foo"

Also, conditional comments of the following form are now supported:

 comment "Isn't foo great?"
      depends on FOO

And support was added for comments following dependencies like:

      depends on FOO # We all depend on FOO

Impact

Should have no impact since this is not a critical build tool.

NOTE: There is one other, unrelated issues:

$ tools/mkconfigvars.sh
Unrecognized garbage after default value

This is due to missing logic to handle default values that are expressions. Specifically, the problem is caused by this in graphics/Kconfig:

    config NX_UPDATE
      bool "Display update hooks"
      default FB_UPDATE && !NX_LCDDRIVER

This is not currently addressed by this PR.

Testing

Tested using tools/mkconfigvars.sh

patacongo avatar Apr 26 '21 18:04 patacongo

This is marked a draft because additional effort will be required to get this through nxstyle, mostly because there are many long lines reported. This PR does at least provide a fix to the problem if this ever becomes an issue for anyone who is more motivated than I to deal with the nxstyle complaints.

patacongo avatar Apr 26 '21 18:04 patacongo

File ./arch/risc-v/src/esp32c3/Kconfig Unhandled token: depends

This error is related to this comment command:

comment "Selected ESP32-C3 chip without embedded Flash, an external Flash memory is required."
	depends on ARCH_CHIP_ESP32C3X

arch/xtensa/src/esp32/Kconfig contains a similar comment to let the user know why some options are hidden.

gustavonihei avatar Apr 27 '21 19:04 gustavonihei

File ./arch/risc-v/src/esp32c3/Kconfig Unhandled token: depends

This error is related to this comment command:

comment "Selected ESP32-C3 chip without embedded Flash, an external Flash memory is required."
	depends on ARCH_CHIP_ESP32C3X

arch/xtensa/src/esp32/Kconfig contains a similar comment to let the user know why some options are hidden.

Thanks. I didn't look at all of the of depends on in the file. When I new which one it was the fix was easy. We talked about this in the past and decided not to support condition comments of that form, that is why it is not support. The ones for the Expressif parts are inconsistent and the only cases where that form is used. Those comments probably should be removed (I know the Linux community encourages that usage, but it has been discourage in NuttX for many years so any usage is inconsistent).

BTW: The other

depends on ARCH_CHIP_ESP32C3

are unnecessary and redundant since the Kconfig file begins with:

if ARCH_CHIP_ESP32C3

Everything from there to the matching endif has the that dependency.

patacongo avatar Apr 27 '21 20:04 patacongo

I have not looked into the other reported error, but here is a little more information from running within debug output enabled:

      <li><a href="#CONFIG_ESP32C3_UART1_RXPIN">1.2.717.4 <code>CONFIG_ESP32C3_UART1_RXPIN</code>: UART1 RX Pin</a></li>
      <li><a href="#CONFIG_ESP32C3_PARTITION">1.2.717.5 <code>CONFIG_ESP32C3_PARTITION</code>: ESP32-C3 Partition</a></li>
    </ul>
    Menu: Terminating token: config
    <li><a name="menu_365_toc"><a href="#menu_365">1.2.718 Menu: Real-Time Timer</a></a></li>
<a href="#menu_365_toc" onclick="toggle('toggle_364', this)">Expand</a>
    <ul id="toggle_364"  style="display:none">
    process_menu: TOKEN_MENU
      kconfigdir:  ./arch/risc-v/src/esp32c3
      kconfigname: Kconfig
      level:       4
    CONFIG_ESP32C3_RT_TIMER_TASK_NAME: Terminating token: config
    <li><a href="#CONFIG_ESP32C3_RT_TIMER_TASK_NAME">1.2.718.1 <code>CONFIG_ESP32C3_RT_TIMER_TASK_NAME</code>: Timer task name</a></li>
    Unrecognized garbage after default value

It is probably complaining about this:

  default 223 # Lower than high priority workqueue

patacongo avatar Apr 27 '21 20:04 patacongo

Thanks. I didn't look at all of the of depends on in the file. When I new which one it was the fix was easy. We talked about this in the past and decided not to support condition comments of that form, that is why it is not support. The ones for the Expressif parts are inconsistent and the only cases where that form is used. Those comments probably should be removed (I know the Linux community encourages that usage, but it has been discourage in NuttX for many years so any usage is inconsistent).

No problem, I can submit a PR to remove the comments. I was used to working with Buildroot and I thought it could be interesting to add.

BTW: The other

depends on ARCH_CHIP_ESP32C3

are unnecessary and redundant since the Kconfig file begins with:

if ARCH_CHIP_ESP32C3

Everything from there to the matching endif has the that dependency.

Thanks for the tip, I'll address that as well.

gustavonihei avatar Apr 27 '21 21:04 gustavonihei

No problem, I can submit a PR to remove the comments. I was used to working with Buildroot and I thought it could be interesting to add.

No problem. I updated the tool skip over # comments at the end of the line.

Thanks for the tip, I'll address that as well.

There are other issues as well. For example, people have begun using default values that are expressions of other configuration settings. The tool would not need to parse the expressions other than to know when they end.

But clearly this tools has not been used for many years and it would take more effort to get it back "up to snuff" and passing nxstyle issues. If the tools is truly unused, then should just be deleted rather than supported and fixed.

All of the other documentation has moved away from HTML.

patacongo avatar Apr 27 '21 21:04 patacongo