* [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE @ 2012-05-21 15:53 Alan Ott 2012-05-21 20:06 ` Sascha Hauer 2012-05-21 22:44 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 2 replies; 11+ messages in thread From: Alan Ott @ 2012-05-21 15:53 UTC (permalink / raw) To: barebox; +Cc: Alan Ott print_menu_entry() in common/menu.c depeneds on process_escape_sequence(). Signed-off-by: Alan Ott <alan@signal11.us> --- common/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/common/Kconfig b/common/Kconfig index 73d620a..ebb9400 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -342,6 +342,7 @@ config AUTO_COMPLETE config MENU bool prompt "Menu Framework" + depends on PROCESS_ESCAPE_SEQUENCE help a menu framework that allow us to create list menu to simplify barebox and make it more user-frendly -- 1.7.0.4 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE 2012-05-21 15:53 [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE Alan Ott @ 2012-05-21 20:06 ` Sascha Hauer 2012-05-22 2:40 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-21 22:44 ` Jean-Christophe PLAGNIOL-VILLARD 1 sibling, 1 reply; 11+ messages in thread From: Sascha Hauer @ 2012-05-21 20:06 UTC (permalink / raw) To: Alan Ott; +Cc: barebox Hi Alan, On Mon, May 21, 2012 at 11:53:36AM -0400, Alan Ott wrote: > print_menu_entry() in common/menu.c depeneds on process_escape_sequence(). > > Signed-off-by: Alan Ott <alan@signal11.us> Applied, thanks Sascha > --- > common/Kconfig | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/common/Kconfig b/common/Kconfig > index 73d620a..ebb9400 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -342,6 +342,7 @@ config AUTO_COMPLETE > config MENU > bool > prompt "Menu Framework" > + depends on PROCESS_ESCAPE_SEQUENCE > help > a menu framework that allow us to create list menu to simplify > barebox and make it more user-frendly > -- > 1.7.0.4 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE 2012-05-21 20:06 ` Sascha Hauer @ 2012-05-22 2:40 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 3:35 ` Alan Ott 0 siblings, 1 reply; 11+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-22 2:40 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox, Alan Ott Hi Please revert break my work on at91bootstrap where we use the menu without any shell support And the does work with or without hush support Best Regards, J. On May 22, 2012, at 4:06 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > Hi Alan, > > On Mon, May 21, 2012 at 11:53:36AM -0400, Alan Ott wrote: >> print_menu_entry() in common/menu.c depeneds on process_escape_sequence(). >> >> Signed-off-by: Alan Ott <alan@signal11.us> > > Applied, thanks > > Sascha > >> --- >> common/Kconfig | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/common/Kconfig b/common/Kconfig >> index 73d620a..ebb9400 100644 >> --- a/common/Kconfig >> +++ b/common/Kconfig >> @@ -342,6 +342,7 @@ config AUTO_COMPLETE >> config MENU >> bool >> prompt "Menu Framework" >> + depends on PROCESS_ESCAPE_SEQUENCE >> help >> a menu framework that allow us to create list menu to simplify >> barebox and make it more user-frendly >> -- >> 1.7.0.4 >> >> >> _______________________________________________ >> barebox mailing list >> barebox@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/barebox >> > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE 2012-05-22 2:40 ` Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-22 3:35 ` Alan Ott 0 siblings, 0 replies; 11+ messages in thread From: Alan Ott @ 2012-05-22 3:35 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On 05/21/2012 10:40 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > Hi > > Please revert break my work on at91bootstrap where we use the menu without any shell support > > And the does work with or without hush support Upon closer inspection, it should be: config MENU bool prompt "Menu Framework" + select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH I have posted v2 which does this, and I agree that the original patch should be reverted in favor of the new one. Steps to reproduce on the master branch: 1. build on amd64 (no ARCH= or CROSS_COMPILE= or anything else) 2. make distclean 3. make menuconfig 4. Select "Menu Framework" from General Settings 5. leave everything else default. 6. make 7. output at [1] (link error). Alan. [1] LD barebox common/built-in.o: In function `print_menu_entry': menu.c:(.text+0x5415): undefined reference to `process_escape_sequence' common/built-in.o: In function `print_menu': menu.c:(.text+0x54a3): undefined reference to `process_escape_sequence' collect2: ld returned 1 exit status make: *** [barebox] Error 1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE 2012-05-21 15:53 [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE Alan Ott 2012-05-21 20:06 ` Sascha Hauer @ 2012-05-21 22:44 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 2:22 ` [PATCH] common: Make HUSH_SHELL select PROCESS_ESCAPE_SEQUENCE Alan Ott 2012-05-22 2:24 ` [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE Alan Ott 1 sibling, 2 replies; 11+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-21 22:44 UTC (permalink / raw) To: Alan Ott; +Cc: barebox On 11:53 Mon 21 May , Alan Ott wrote: > print_menu_entry() in common/menu.c depeneds on process_escape_sequence(). > > Signed-off-by: Alan Ott <alan@signal11.us> > --- > common/Kconfig | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/common/Kconfig b/common/Kconfig > index 73d620a..ebb9400 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -342,6 +342,7 @@ config AUTO_COMPLETE > config MENU > bool > prompt "Menu Framework" > + depends on PROCESS_ESCAPE_SEQUENCE > help > a menu framework that allow us to create list menu to simplify > barebox and make it more user-frendly no the menu does not need this as I use it withouth hush Best Regards, J. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] common: Make HUSH_SHELL select PROCESS_ESCAPE_SEQUENCE 2012-05-21 22:44 ` Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-22 2:22 ` Alan Ott 2012-05-22 3:32 ` [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected Alan Ott 2012-05-22 2:24 ` [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE Alan Ott 1 sibling, 1 reply; 11+ messages in thread From: Alan Ott @ 2012-05-22 2:22 UTC (permalink / raw) To: plagnioj; +Cc: barebox, Alan Ott Possibly among other things, print_menu_entry() in common/menu.c calls on process_escape_sequence() when HUSH_SHELL is selected. Signed-off-by: Alan Ott <alan@signal11.us> --- common/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/common/Kconfig b/common/Kconfig index 73d620a..7aa50b6 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -280,6 +280,7 @@ choice select COMMAND_SUPPORT select PARAMETER select BINFMT + select PROCESS_ESCAPE_SEQUENCE help Enable hush support. This is the most advanced shell available for barebox. -- 1.7.0.4 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected 2012-05-22 2:22 ` [PATCH] common: Make HUSH_SHELL select PROCESS_ESCAPE_SEQUENCE Alan Ott @ 2012-05-22 3:32 ` Alan Ott 2012-05-22 9:27 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 11+ messages in thread From: Alan Ott @ 2012-05-22 3:32 UTC (permalink / raw) To: plagnioj; +Cc: barebox, Alan Ott print_menu_entry() in common/menu.c calls on process_escape_sequence() when HUSH_SHELL is selected. Signed-off-by: Alan Ott <alan@signal11.us> --- common/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/common/Kconfig b/common/Kconfig index 73d620a..7cd0666 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -342,6 +342,7 @@ config AUTO_COMPLETE config MENU bool prompt "Menu Framework" + select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH help a menu framework that allow us to create list menu to simplify barebox and make it more user-frendly -- 1.7.0.4 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected 2012-05-22 3:32 ` [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected Alan Ott @ 2012-05-22 9:27 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 15:08 ` Alan Ott 0 siblings, 1 reply; 11+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-22 9:27 UTC (permalink / raw) To: Alan Ott; +Cc: barebox On 23:32 Mon 21 May , Alan Ott wrote: > print_menu_entry() in common/menu.c calls on process_escape_sequence() when > HUSH_SHELL is selected. > > Signed-off-by: Alan Ott <alan@signal11.us> > --- > common/Kconfig | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/common/Kconfig b/common/Kconfig > index 73d620a..7cd0666 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -342,6 +342,7 @@ config AUTO_COMPLETE > config MENU > bool > prompt "Menu Framework" > + select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH > help > a menu framework that allow us to create list menu to simplify > barebox and make it more user-frendly you miss the point of the code the PROCESS_ESCAPE_SEQUENCE is optionnal and need to stay optionnal evenif Hush is enable Best Regards, J. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected 2012-05-22 9:27 ` Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-22 15:08 ` Alan Ott 2012-05-22 19:04 ` Sascha Hauer 0 siblings, 1 reply; 11+ messages in thread From: Alan Ott @ 2012-05-22 15:08 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On 05/22/2012 05:27 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 23:32 Mon 21 May , Alan Ott wrote: >> print_menu_entry() in common/menu.c calls on process_escape_sequence() when >> HUSH_SHELL is selected. >> >> Signed-off-by: Alan Ott <alan@signal11.us> >> --- >> common/Kconfig | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/common/Kconfig b/common/Kconfig >> index 73d620a..7cd0666 100644 >> --- a/common/Kconfig >> +++ b/common/Kconfig >> @@ -342,6 +342,7 @@ config AUTO_COMPLETE >> config MENU >> bool >> prompt "Menu Framework" >> + select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH >> help >> a menu framework that allow us to create list menu to simplify >> barebox and make it more user-frendly > you miss the point of the code > > the PROCESS_ESCAPE_SEQUENCE is optionnal and need to stay optionnal > evenif Hush is enable It's required if both HUSH_SHELL _and_ MENU are selected. The code in common/menu.c looks like this: if (IS_ENABLED(CONFIG_SHELL_HUSH)) { process_escape_sequence(m->display, m->display_buffer, m->display_buffer_size); puts(m->display_buffer); } else { puts(m->display); } Thus, the configuration needs to say, if you turn on MENU (enabling menu.c), then select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is also selected (because it will try to call process_escape_sequence()). As I described in my other email, it's very easy to create a test case which does not build. Keep in mind, I'm talking about making a Kconfig which is correct for the code as the code currently is. Maybe in the opinion of some (you?), it is the code that should be different, not the configuration. If you or someone else has a better idea how to fix this, that would be fine too. Right now however, it is broken. Alan. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected 2012-05-22 15:08 ` Alan Ott @ 2012-05-22 19:04 ` Sascha Hauer 0 siblings, 0 replies; 11+ messages in thread From: Sascha Hauer @ 2012-05-22 19:04 UTC (permalink / raw) To: Alan Ott; +Cc: barebox On Tue, May 22, 2012 at 11:08:51AM -0400, Alan Ott wrote: > On 05/22/2012 05:27 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On 23:32 Mon 21 May , Alan Ott wrote: > >> print_menu_entry() in common/menu.c calls on process_escape_sequence() when > >> HUSH_SHELL is selected. > >> > >> Signed-off-by: Alan Ott <alan@signal11.us> > >> --- > >> common/Kconfig | 1 + > >> 1 files changed, 1 insertions(+), 0 deletions(-) > >> > >> diff --git a/common/Kconfig b/common/Kconfig > >> index 73d620a..7cd0666 100644 > >> --- a/common/Kconfig > >> +++ b/common/Kconfig > >> @@ -342,6 +342,7 @@ config AUTO_COMPLETE > >> config MENU > >> bool > >> prompt "Menu Framework" > >> + select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH > >> help > >> a menu framework that allow us to create list menu to simplify > >> barebox and make it more user-frendly > > you miss the point of the code > > > > the PROCESS_ESCAPE_SEQUENCE is optionnal and need to stay optionnal > > evenif Hush is enable > > It's required if both HUSH_SHELL _and_ MENU are selected. The code in > common/menu.c looks like this: > > if (IS_ENABLED(CONFIG_SHELL_HUSH)) { > process_escape_sequence(m->display, > m->display_buffer, > m->display_buffer_size); > puts(m->display_buffer); > } else { > puts(m->display); > } From what I see your v2 patch is correct. Another possibility would maybe be replacing the if above with IS_ENABLED(CONFIG_PROCESS_ESCAPE_SEQUENCE), but I don't know if that would produce correct code. Jean-Christophe? Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE 2012-05-21 22:44 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 2:22 ` [PATCH] common: Make HUSH_SHELL select PROCESS_ESCAPE_SEQUENCE Alan Ott @ 2012-05-22 2:24 ` Alan Ott 1 sibling, 0 replies; 11+ messages in thread From: Alan Ott @ 2012-05-22 2:24 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On 05/21/2012 06:44 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 11:53 Mon 21 May , Alan Ott wrote: >> print_menu_entry() in common/menu.c depeneds on process_escape_sequence(). >> >> Signed-off-by: Alan Ott <alan@signal11.us> >> --- >> common/Kconfig | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/common/Kconfig b/common/Kconfig >> index 73d620a..ebb9400 100644 >> --- a/common/Kconfig >> +++ b/common/Kconfig >> @@ -342,6 +342,7 @@ config AUTO_COMPLETE >> config MENU >> bool >> prompt "Menu Framework" >> + depends on PROCESS_ESCAPE_SEQUENCE >> help >> a menu framework that allow us to create list menu to simplify >> barebox and make it more user-frendly > no the menu does not need this as I use it withouth hush > Jean-Christophe, You're right. I just posted another patch which may work better. Thanks! Alan. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-05-22 19:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-05-21 15:53 [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE Alan Ott 2012-05-21 20:06 ` Sascha Hauer 2012-05-22 2:40 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 3:35 ` Alan Ott 2012-05-21 22:44 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 2:22 ` [PATCH] common: Make HUSH_SHELL select PROCESS_ESCAPE_SEQUENCE Alan Ott 2012-05-22 3:32 ` [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected Alan Ott 2012-05-22 9:27 ` Jean-Christophe PLAGNIOL-VILLARD 2012-05-22 15:08 ` Alan Ott 2012-05-22 19:04 ` Sascha Hauer 2012-05-22 2:24 ` [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE Alan Ott
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox