| Message ID | 20180130181853.32512-1-ulfalizer@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2018-01-30 at 19:18 +0100, Ulf Magnusson wrote: > Print a warning if a 'help' token is given but the help text is blank. > Personal pet peeve. > > Example warnings: > > net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank help text > net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank help text > net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank help text > drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with blank help text > drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with blank help text > drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with blank help text > drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' defined with blank help text > lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with blank help text > > A separate patchset will be sent to fix all current instances of blank > help texts for all arches. I added the same warning to Kconfiglib. If you do this it would be better to first fix or remove those help texts, and only then add this warning. Ie, add the warning in the last patch of a cleanup series. > --- a/scripts/kconfig/zconf.y > +++ b/scripts/kconfig/zconf.y > @@ -436,6 +436,12 @@ help: help_start T_HELPTEXT > zconfprint("warning: '%s' defined with more than one help text -- only the last one will be used", > current_entry->sym->name ?: "<choice>"); > } > + > + /* Is the help text empty or all whitespace? */ > + if ($2[strspn($2, " \f\n\r\t\v")] == '\0') > + zconfprint("warning: '%s' defined with blank help text", > + current_entry->sym->name ?: "<choice>"); > + Does this go to stderr? Another fix would be to ignore empty help texts and not render them at all. Is that possible? Thanks, Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 30, 2018 at 7:34 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Tue, 2018-01-30 at 19:18 +0100, Ulf Magnusson wrote: >> Print a warning if a 'help' token is given but the help text is blank. >> Personal pet peeve. >> >> Example warnings: >> >> net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank help text >> net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank help text >> net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank help text >> drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with blank help text >> drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with blank help text >> drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with blank help text >> drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' defined with blank help text >> lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with blank help text >> >> A separate patchset will be sent to fix all current instances of blank >> help texts for all arches. I added the same warning to Kconfiglib. > > If you do this it would be better to first fix or remove those help texts I agree that this shouldn't go in until/unless a significant portion of those empty help texts get removed first. > and only then add this warning. Ie, add the warning in the last patch of a cleanup > series. Might have been better to arrange it like that, yeah. The patchset that removes the empty help texts is at https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd prefer that. > >> --- a/scripts/kconfig/zconf.y >> +++ b/scripts/kconfig/zconf.y >> @@ -436,6 +436,12 @@ help: help_start T_HELPTEXT >> zconfprint("warning: '%s' defined with more than one help text -- only the last one will be used", >> current_entry->sym->name ?: "<choice>"); >> } >> + >> + /* Is the help text empty or all whitespace? */ >> + if ($2[strspn($2, " \f\n\r\t\v")] == '\0') >> + zconfprint("warning: '%s' defined with blank help text", >> + current_entry->sym->name ?: "<choice>"); >> + > > Does this go to stderr? Yep - goes to stderr. zconfprint() is the standard error reporting function during parsing, and also prints the parsing location. > > Another fix would be to ignore empty help texts and not render them at all. Is > that possible? I haven't even checked how they are rendered to be honest. I was more concerned with the Kconfig mess. > Thanks, > > > Paul Bolle Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2018-01-30 at 20:33 +0100, Ulf Magnusson wrote: > I agree that this shouldn't go in until/unless a significant portion > of those empty help texts get removed first. Not a significant portion, but all, I'd say. > The patchset that removes the empty help texts is at > https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd > prefer that. I'm fine with anything that doesn't add warnings just to appease pet peeves. > I haven't even checked how they are rendered to be honest. I was more > concerned with the Kconfig mess. Well, if you could make empty help texts disappear somehow (either during the Kconfig parse phase - which requires a jump into the yacc horror - or during the rendering phase) that would be much better than adding a warning for something that you yourself described as a pet peeve. And after that's done you're free to send cleanup patches for this whenever you feel like doing so. Thanks, Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 30, 2018 at 9:31 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Tue, 2018-01-30 at 20:33 +0100, Ulf Magnusson wrote: >> I agree that this shouldn't go in until/unless a significant portion >> of those empty help texts get removed first. > > Not a significant portion, but all, I'd say. > >> The patchset that removes the empty help texts is at >> https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd >> prefer that. > > I'm fine with anything that doesn't add warnings just to appease pet peeves. > >> I haven't even checked how they are rendered to be honest. I was more >> concerned with the Kconfig mess. > > Well, if you could make empty help texts disappear somehow (either during the > Kconfig parse phase - which requires a jump into the yacc horror - or during > the rendering phase) that would be much better than adding a warning for > something that you yourself described as a pet peeve. > > And after that's done you're free to send cleanup patches for this whenever > you feel like doing so. > > Thanks, > > > Paul Bolle I rolled a new patchset which adds the warning at the end and makes the commits that remove the blank help texts more freestanding: https://lkml.org/lkml/2018/1/31/137 Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y index 21ce883e5d9e..4be98050b961 100644 --- a/scripts/kconfig/zconf.y +++ b/scripts/kconfig/zconf.y @@ -436,6 +436,12 @@ help: help_start T_HELPTEXT zconfprint("warning: '%s' defined with more than one help text -- only the last one will be used", current_entry->sym->name ?: "<choice>"); } + + /* Is the help text empty or all whitespace? */ + if ($2[strspn($2, " \f\n\r\t\v")] == '\0') + zconfprint("warning: '%s' defined with blank help text", + current_entry->sym->name ?: "<choice>"); + current_entry->help = $2; };
Print a warning if a 'help' token is given but the help text is blank. Personal pet peeve. Example warnings: net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank help text net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank help text net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank help text drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with blank help text drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with blank help text drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with blank help text drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' defined with blank help text lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with blank help text A separate patchset will be sent to fix all current instances of blank help texts for all arches. I added the same warning to Kconfiglib. Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com> --- scripts/kconfig/zconf.y | 6 ++++++ 1 file changed, 6 insertions(+)