Skip to content
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

Feature Request: Call back previous commands in interactive mode #57

Closed
whereswaldon opened this issue Nov 30, 2016 · 17 comments
Closed

Comments

@whereswaldon
Copy link

I believe that this repo is the code that powers go tool pprof. If I'm wrong on that count, disregard everything else I say.

I'm using pprof on macOS 10.12.1 with go 1.7.3. I'm constantly using it in interactive mode, and it would be really nice to be able to call back previous commands with the "up" arrow key. Instead, of course, it prints ^[[A, which I assume means that this isn't supported?

I completely understand if this isn't worth the development team's time, but I thought I would suggest it.

@dgryski
Copy link
Contributor

dgryski commented Nov 30, 2016

There are a number of pure-Go readline implementations that could be integrated to allow this.

@rauls5382
Copy link
Contributor

This is the upstream repo for the pprof tool. The one in the Go distribution is an old snapshot of this codebase. We plan to eventually refresh it to a more recent version, I believe in Go 1.9

@dgryski : Any pointers to specific readline implementations? Are there any in use in the Go distribution?

@aalexand aalexand assigned rauls5382 and unassigned rauls5382 Dec 14, 2016
@rauls5382
Copy link
Contributor

I have an initial prototype for this using github.com/chzyer/readline at
https://github.com/rauls5382/pprof/tree/readline

Would you like to try it out and see if it is useful? It lets you recall and edit commands from the current session, but it doesn't provide a persistent history across sessions. Would you find this useful, or is cross-session history the most useful part?

@whereswaldon
Copy link
Author

@rauls5382 Thanks for trying to build it! The workflow that I usually want this feature for is calling back the last command to add some filters. While cross-session history might be useful, it wasn't my goal.

Sadly, I'm having some trouble running it. It compiles fine, but I can't open a profile with it. That said, I also can't open a profile with master. Has the profile format changed since the one shipping as go tool pprof with go 1.6.2? What do I need to do to record a memory profile in the format that this tool expects?

@rauls5382
Copy link
Contributor

No, pprof should still be able to read profiles from older versions. On Go 1.8 now the handlers generate a native profile, but pprof will continue to read profiles from older versions.

If you can, attach a profile here and I can take a look. Did you collect it directly from the URL, or did you use pprof to fetch/save it?

@whereswaldon
Copy link
Author

@rauls5382 I'm sorry, I did something stupid, and the profile file was empty. Anyway, it works ☁️ wonderfully ☁️. This is exactly what I wanted. Thank you so much! Is there any obstacle to adding this to master?

@rauls5382
Copy link
Contributor

There are some concerns with adding the new dependence to master. I'll work on it but it may take some time. Feel free to continue using my branch until then.

@whereswaldon
Copy link
Author

Okay, thank you!

@levex
Copy link

levex commented Aug 4, 2017

Any news on this @rauls5382 ? I'd be happy to take on if you can't continue.

As a workaround, rlwrap seems to be okay for now.

@ALTree
Copy link
Contributor

ALTree commented Jan 22, 2018

@aalexand @nolanmar511 thoughts on this? Adding readline support should be relatively easy if some pure-go readline implementation is vendored; I guess the question here is whether it's fine to vendor a readline implementation or not.

@aalexand
Copy link
Collaborator

aalexand commented Jan 23, 2018

@ALTree pprof is vendored into Golang source tree so I am not enthusiastic about adding a new dependency for this feature request. I wonder if it should be relatively easy to implement some limited readline-like-support without adding a new dependency.

@aalexand
Copy link
Collaborator

Could we use https://github.com/golang/crypto/blob/master/ssh/terminal/terminal.go? That seems to have limited history support. Is that less problematic to depend on in golang land?

@pedrosland
Copy link

I don't know if there is a desire to implement more readline features in the future but I would be very happy with only history support and just for the current session.

If @rauls5382 or @levex are busy, I would be willing to pick this up and integrate crypto's terminal.go or just history.

@aalexand
Copy link
Collaborator

aalexand commented Apr 7, 2018

@rsc @hyangah @mvdan Do you see any issues (in terms of vendoring pprof into Go source tree) with pprof depending on https://github.com/golang/crypto/blob/master/ssh/terminal/terminal.go to implement limited history and editing support in the interactive mode?

@mvdan
Copy link
Contributor

mvdan commented Apr 7, 2018

I don't know what the policy is around how much code can be vendored into the Go repository, sorry. I imagine Russ or other members of the Go team will have an opinion on it.

@andybons
Copy link

Regarding vendoring in the terminal stuff, go for it.

@rauls5382
Copy link
Contributor

I'm sorry for the long silence on this. I think we can merge in my previous prototype while maintaining the existing version by default. I'll find some time to work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants