New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 777566 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

crosh help missing lines for top and help_advanced

Project Member Reported by jamescook@chromium.org, Oct 23 2017

Issue description

Tested on caroline on M63 dev (63.0.3239.7, 10032.4.0)

* Hit Ctrl-Alt-T for crosh
* Type "help"

Help appears for all the commands. However, 2 lines are missing, with the command name for help_advanced and top.

See screenshot. I'm not sure if this is a rendering problem or if it's just not printing the lines.

I see similar glitches when typing "help_advanced" (missing lines in the output). Maybe it's hterm (is that what we use for crosh?).

 
crosh help missing lines.png
374 KB View Download

Comment 1 by vapier@chromium.org, Oct 23 2017

Cc: briannorris@chromium.org vbendeb@chromium.org
Owner: vapier@chromium.org
Status: Started (was: Unconfirmed)
i've posted CLs for this already.  lemme finish up the feedback.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7c6a07946078fa2604559ee66f5d97950a73301e

commit 7c6a07946078fa2604559ee66f5d97950a73301e
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Oct 24 05:39:09 2017

crosh: fix --dash handling

When we added the main loop parser, we didn't ignore the existing
--dash argument.

BUG= chromium:777566 
TEST=`./crosh --dash` works again

Change-Id: I8002f4ac78d4a5ac5b38024828d2dee55e02aedd
Reviewed-on: https://chromium-review.googlesource.com/733613
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/7c6a07946078fa2604559ee66f5d97950a73301e/crosh/crosh

Comment 3 by vapier@chromium.org, Oct 24 2017

Components: -Platform>Apps>Default>Hterm OS>Systems
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3e2d7d543482599b5c9a1602d89ac396cdc770ad

commit 3e2d7d543482599b5c9a1602d89ac396cdc770ad
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Oct 24 09:20:33 2017

crosh: fix command enumeration in help_advanced

The help refactoring work in CL:671877 broke the method help_advanced
used to discover registered commands.  We already have some code that
discovers all registered commands, so put that in a dedicated func we
can re-use here.

Expand the existing documentation so hopefully the interactions between
all these variables remains clear and prevents future regressions.

BUG= chromium:777566 
TEST=`help_advanced` shows wpa_debug and uptime again

Change-Id: Ibc6defaa9921272353da87700b1558f40ed40f78
Reviewed-on: https://chromium-review.googlesource.com/728946
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/3e2d7d543482599b5c9a1602d89ac396cdc770ad/crosh/crosh
[modify] https://crrev.com/3e2d7d543482599b5c9a1602d89ac396cdc770ad/crosh/README.md

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/d04202fee177b54c3b8c352a7c4ca9f8e35edfe6

commit d04202fee177b54c3b8c352a7c4ca9f8e35edfe6
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Oct 24 09:20:33 2017

crosh: fix help output with commands that take no arguments

In CL:671877, I was trying to support commands that defined both a custom
help function and custom HELP/USAGE strings.  In reality, we don't really
have any such commands, it doesn't seem likely we will start, and the code
currently omits commands that should be shown.

Keep the two code paths separate so it's easier to reason about.  Having
too much flexibility in crosh is a bad thing too as a shell script we run
in verified mode.

BUG= chromium:777566 
TEST=`help` shows help_advanced again
TEST=`help_advanced` shows wpa_debug and uptime again (but omits ssh still)

Change-Id: Idedeca93f33398bbc94a8eb084b86828f040638c
Reviewed-on: https://chromium-review.googlesource.com/728947
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/d04202fee177b54c3b8c352a7c4ca9f8e35edfe6/crosh/crosh

Comment 6 by vapier@chromium.org, Oct 24 2017

Labels: Merge-Request-63
risk should be very low as it only affects crosh

Comment 7 by gkihumba@google.com, Oct 25 2017

Labels: -Merge-Request-63 Merge-Approved-63
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 26 2017

Labels: merge-merged-release-R63-10032.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3ff1375050acf7d92f95e463808f075e3cf4d9c2

commit 3ff1375050acf7d92f95e463808f075e3cf4d9c2
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Oct 26 01:36:23 2017

crosh: fix help output with commands that take no arguments

In CL:671877, I was trying to support commands that defined both a custom
help function and custom HELP/USAGE strings.  In reality, we don't really
have any such commands, it doesn't seem likely we will start, and the code
currently omits commands that should be shown.

Keep the two code paths separate so it's easier to reason about.  Having
too much flexibility in crosh is a bad thing too as a shell script we run
in verified mode.

BUG= chromium:777566 
TEST=`help` shows help_advanced again
TEST=`help_advanced` shows wpa_debug and uptime again (but omits ssh still)

Change-Id: Idedeca93f33398bbc94a8eb084b86828f040638c
(cherry picked from commit d04202fee177b54c3b8c352a7c4ca9f8e35edfe6)
Reviewed-on: https://chromium-review.googlesource.com/738271
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/3ff1375050acf7d92f95e463808f075e3cf4d9c2/crosh/crosh

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/25082c1d2193fe22978aec7bfa990b09f78ec0c8

commit 25082c1d2193fe22978aec7bfa990b09f78ec0c8
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Oct 26 01:36:27 2017

crosh: fix command enumeration in help_advanced

The help refactoring work in CL:671877 broke the method help_advanced
used to discover registered commands.  We already have some code that
discovers all registered commands, so put that in a dedicated func we
can re-use here.

Expand the existing documentation so hopefully the interactions between
all these variables remains clear and prevents future regressions.

BUG= chromium:777566 
TEST=`help_advanced` shows wpa_debug and uptime again

Change-Id: Ibc6defaa9921272353da87700b1558f40ed40f78
(cherry picked from commit 3e2d7d543482599b5c9a1602d89ac396cdc770ad)
Reviewed-on: https://chromium-review.googlesource.com/738270
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/25082c1d2193fe22978aec7bfa990b09f78ec0c8/crosh/crosh
[modify] https://crrev.com/25082c1d2193fe22978aec7bfa990b09f78ec0c8/crosh/README.md

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/5d370571166ad7dd77bd25ca34fc6ae0f571e875

commit 5d370571166ad7dd77bd25ca34fc6ae0f571e875
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Oct 26 01:36:29 2017

crosh: fix --dash handling

When we added the main loop parser, we didn't ignore the existing
--dash argument.

BUG= chromium:777566 
TEST=`./crosh --dash` works again

Change-Id: I8002f4ac78d4a5ac5b38024828d2dee55e02aedd
(cherry picked from commit 7c6a07946078fa2604559ee66f5d97950a73301e)
Reviewed-on: https://chromium-review.googlesource.com/738269
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/5d370571166ad7dd77bd25ca34fc6ae0f571e875/crosh/crosh

Labels: -Merge-Approved-63 Merge-Merged
Status: Fixed (was: Started)

Sign in to add a comment