Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions internal/driver/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (<x> 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(
Expand Down Expand Up @@ -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])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: why did 16 change to 22?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To accommodate extra texts (e.g. =) added to the help message and really long option name (i.e. relative_percentages)

}

var commands []string
Expand Down Expand Up @@ -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 += " = <n>"
} else if vr.kind == floatKind {
option += " = <f>"
} else if name == "sample_index" {
option += " = <x>"
}

variables = append(variables, fmtHelp(option, vr.help))
}
sort.Strings(variables)

Expand Down
8 changes: 8 additions & 0 deletions internal/driver/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"did you mean"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use backtick string to avoid having to escape double quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need %v here? Would %s be sufficient? Avoid %v unless required, it can leak implementation details in a funny way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
return nil, nil, fmt.Errorf("unrecognized command: %q", name)
}

Expand Down
15 changes: 14 additions & 1 deletion internal/driver/interactive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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",
}

Expand Down