New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/pprof: add readline support #14041

Open
valyala opened this Issue Jan 20, 2016 · 4 comments

Comments

Projects
None yet
6 participants
@valyala
Contributor

valyala commented Jan 20, 2016

Currently go tool pprof in interactive mode doesn't support readline features such as command history, ctrl-r, tab-completion, etc. It would be great to add such functionality.

Basic integration is quite easy - see the gist containing PoC readline patch for go tool pprof. It is based on pure go implementation of readline.

@bradfitz bradfitz changed the title from Feature request: add readline support for `go tool pprof` interactive mode to cmd/pprof: add readline support Jan 20, 2016

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Jan 20, 2016

@ALTree

This comment has been minimized.

Member

ALTree commented Jan 22, 2018

upstream issue: google/pprof/issues/57

Unfortunately it seems that the pprof maintainers are reluctant to add an external dependency to the tool.

@hyangah

This comment has been minimized.

Contributor

hyangah commented Apr 11, 2018

The upstream pprof maintainer @aalexand proposed to utilize the golang.org/x/crypto/ssh/terminal package for readline functionality.
Go repo already vendored some packages from x/crypto (https://github.com/golang/go/tree/master/src/vendor/golang_org/x/crypto).
Is it okay to add the ssh/terminal package there?

@gopherbot

This comment has been minimized.

gopherbot commented May 9, 2018

Change https://golang.org/cl/112436 mentions this issue: cmd/pprof: provides readline feature

@hyangah

This comment has been minimized.

Contributor

hyangah commented May 9, 2018

cl/112436 implements the feature requested here. I was ok'd about vendoring x/crypto/ssh/terminal but found it depends on some packages in x/sys as well. They bring in a lot more files than I hoped and I am currently fighting for all kinds of build/vet errors discovered while vendoring. Already 1.11 dev tree is closed and I am afraid it's too late.

@agnivade agnivade added NeedsFix and removed NeedsDecision labels May 10, 2018

@agnivade agnivade modified the milestones: Unplanned, Go1.12 May 10, 2018

hyangah added a commit to hyangah/pprof that referenced this issue May 21, 2018

Make readlineUI the default UI
The current implementation placed the dependency on
github.com/chzyer/readline in pprof.go instead of placing
it in the default ui just because the go repository that
vendors pprof may not want the dependency.

I evaluated another option (golang.org/x/crypto/ssh/terminal)
for the Go repository and there exist subtle differences
in the behavior. Inconsistency in UI is undesirable so golang
decided to vendor the chzyer readline as well for now.

Update golang/go#14041

gopherbot pushed a commit that referenced this issue May 23, 2018

cmd/pprof: add readline support similar to upstream
The upstream pprof implements the readline feature using
the github.com/chzyer/readline package in its pprof.go main.

It would be ideal to use the same readline support package as
the upstream for better user experience and code maintenance.
However, bringing in third-party packages requires more work
than I envisioned (e.g. clean up the vendored code to meet the
expected standard - iow don't break builders).

As a result, this change implements the similar feature
for the pprof command included in the go distribution
(cmd/pprof/pprof.go) using golang.org/x/crypto/ssh/terminal
for now.

Auto-completion is not yet supported (same in the upstream).

The feature is enabled only in linux, windows, darwin, and
only when terminal support is available.

This change brings in new vendored packages,
golang.org/x/crypto/ssh/terminal and
golang.org/x/sys/{unix,windows}.

For #14041

Change-Id: If4a790796acf2ab20f7e81268b9d9354c5a5cd2b
Reviewed-on: https://go-review.googlesource.com/112436
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

@ianlancetaylor ianlancetaylor modified the milestones: Go1.12, Unplanned Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment