From a9ea9082c8e07243edfaf027b58aea673841ad24 Mon Sep 17 00:00:00 2001 From: Garrett Wang Date: Wed, 8 Apr 2020 14:53:41 -0700 Subject: [PATCH 01/18] Provide more helpful error message and require input for every non-bool option. --- internal/driver/commands.go | 15 ++++++++++++--- internal/driver/interactive.go | 8 ++++++++ internal/driver/interactive_test.go | 15 ++++++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/internal/driver/commands.go b/internal/driver/commands.go index f52471490..a6cb3ad39 100644 --- a/internal/driver/commands.go +++ b/internal/driver/commands.go @@ -215,7 +215,7 @@ var pprofVariables = variables{ "For memory profiles, report average memory per allocation.", "For time-based profiles, report average time per event.")}, "sample_index": &variable{stringKind, "", "", helpText( - "Sample value to report (0-based index or name)", + "Sample value to report ( is 0-based index or name)", "Profiles contain multiple values per sample.", "Use sample_index=i to select the ith value (starting at 0).")}, "normalize": &variable{boolKind, "f", "", helpText( @@ -254,7 +254,7 @@ func usage(commandLine bool) string { prefix = "-" } fmtHelp := func(c, d string) string { - return fmt.Sprintf(" %-16s %s", c, strings.SplitN(d, "\n", 2)[0]) + return fmt.Sprintf(" %-22s %s", c, strings.SplitN(d, "\n", 2)[0]) } var commands []string @@ -284,7 +284,16 @@ func usage(commandLine bool) string { radioOptions[vr.group] = append(radioOptions[vr.group], name) continue } - variables = append(variables, fmtHelp(prefix+name, vr.help)) + option := prefix + name + if vr.kind == intKind { + option += " = " + } else if vr.kind == floatKind { + option += " = " + } else if name == "sample_index" { + option += " = " + } + + variables = append(variables, fmtHelp(option, vr.help)) } sort.Strings(variables) diff --git a/internal/driver/interactive.go b/internal/driver/interactive.go index 3a458b0b7..ea228098b 100644 --- a/internal/driver/interactive.go +++ b/internal/driver/interactive.go @@ -70,6 +70,11 @@ func interactive(p *profile.Profile, o *plugin.Options) error { value = strings.TrimSpace(value) } if v := pprofVariables[name]; v != nil { + // All non-bool options require inputs + if v.kind != boolKind && value == "" { + o.UI.PrintErr(fmt.Errorf("please input a value, e.g. option = value")) + continue + } if name == "sample_index" { // Error check sample_index=xxx to ensure xxx is a valid sample type. index, err := p.SampleIndexByName(value) @@ -267,6 +272,9 @@ func parseCommandLine(input []string) ([]string, variables, error) { } } if c == nil { + if v := pprofVariables[name]; v != nil { + return nil, nil, fmt.Errorf("do you mean: \"%v=%v\"?", name, args[0]) + } return nil, nil, fmt.Errorf("unrecognized command: %q", name) } diff --git a/internal/driver/interactive_test.go b/internal/driver/interactive_test.go index d8e4ef4f3..8341fb2d6 100644 --- a/internal/driver/interactive_test.go +++ b/internal/driver/interactive_test.go @@ -85,6 +85,19 @@ func TestShell(t *testing.T) { if err := interactive(p, o); err != nil { t.Error("invalid group value:", err) } + + // No value provided for the option + pprofVariables = testVariables(savedVariables) + ui = &proftest.TestUI{ + T: t, + Input: []string{"sample_index"}, + AllowRx: `please input a value, e.g. option = value`, + } + o.UI = ui + if err := interactive(p, o); err != nil { + t.Error("no value for the option:", err) + } + // Confirm error message written out once. if ui.NumAllowRxMatches != 1 { t.Errorf("want error message to be printed 1 time, got %v", ui.NumAllowRxMatches) @@ -132,7 +145,7 @@ var script = []string{ "f=-1;f=-2.5;check f=-2.5;f=0.0001;check f=0.0001", "check ff=0;ff=-1.01;check ff=-1.01;ff=100;check ff=100", "s=one;s=two;check s=two", - "ss=tree;check ss=tree;ss=;check ss;ss=forest;check ss=forest", + "ss=tree;check ss=tree;ss=forest;check ss=forest", "ta=true;check ta=true;check tb=false;check tc=false;tb=1;check tb=true;check ta=false;check tc=false;tc=yes;check tb=false;check ta=false;check tc=true", } From a2e56b49caddb67c0de266ef1a2790c59490819f Mon Sep 17 00:00:00 2001 From: Garrett Wang Date: Wed, 8 Apr 2020 17:19:26 -0700 Subject: [PATCH 02/18] Improve the Option groups' help message --- internal/driver/commands.go | 8 +++++--- internal/driver/interactive.go | 11 ++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/driver/commands.go b/internal/driver/commands.go index a6cb3ad39..4ae916d6c 100644 --- a/internal/driver/commands.go +++ b/internal/driver/commands.go @@ -157,7 +157,7 @@ var pprofVariables = variables{ // Filtering options "nodecount": &variable{intKind, "-1", "", helpText( - "Max number of nodes to show", + " is the max number of nodes to show", "Uses heuristics to limit the number of locations to be displayed.", "On graphs, dotted edges represent paths through nodes that have been removed.")}, "nodefraction": &variable{floatKind, "0.005", "", "Hide nodes below *total"}, @@ -208,7 +208,7 @@ var pprofVariables = variables{ "Discard tags that match this regexp")}, // Heap profile options "divide_by": &variable{floatKind, "1", "", helpText( - "Ratio to divide all samples before visualization", + " is the ratio to divide all samples before visualization", "Divide all samples values by a constant, eg the number of processors or jobs.")}, "mean": &variable{boolKind, "f", "", helpText( "Average sample value over first value (count)", @@ -273,7 +273,9 @@ func usage(commandLine bool) string { } help = help + strings.Join(commands, "\n") + "\n\n" + - " Options:\n" + " Options:\n" + + " General format is