console: admin.clearHistory() command #15614
Conversation
|
I have not tested this, but based manual look at the code, I wonder if it actually deletes the history-file from disk ? |
| @@ -216,6 +217,12 @@ func (c *Console) init(preload []string) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (c *Console) clearHistory() { | |||
| c.history = nil | |||
| c.prompter.ClearHistory() | |||
There was a problem hiding this comment.
Please call os.Remove(c.histPath) here.
There was a problem hiding this comment.
done. my original reasoning was that history is being persisted at the end of the session but I agree this is safer.
sorin
force-pushed
the
sorin-console-clear-history
branch
from
0650063 to
271a978
Compare
… sorin-console-clear-history
|
@fjl added a one liner test fix, please approve. thanks! |
| @@ -51,6 +51,9 @@ type UserPrompter interface { | |||
| // if and only if the prompt to append was a valid command. | |||
| AppendHistory(command string) | |||
|
|
|||
| // ClearHistory clears the entire history | |||
| ClearHistory() | |||
There was a problem hiding this comment.
I'd suggest ClearHistory() error, now that the os.remove may return an error
There was a problem hiding this comment.
good suggestion, but since https://github.com/sorin/go-ethereum/blob/sorin-console-clear-history/vendor/github.com/peterh/liner/common.go#L148-L152 does not return an error I'd say ClearHistory() (which is basically a wrapper on top of it) stays like this. Also os.remove is not called inside this ClearHistory() - I did add an error return to Console.clearHistory
| @@ -216,6 +217,13 @@ func (c *Console) init(preload []string) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (c *Console) clearHistory() { | |||
There was a problem hiding this comment.
I'd suggest to let this return an error
console/console.go
Outdated
| func (c *Console) clearHistory() { | ||
| c.history = nil | ||
| c.prompter.ClearHistory() | ||
| os.Remove(c.histPath) |
There was a problem hiding this comment.
... and do return os.Remove(c.histPath)
console/console.go
Outdated
| c.history = nil | ||
| c.prompter.ClearHistory() | ||
| os.Remove(c.histPath) | ||
| fmt.Fprintf(c.printer, "history cleared\n") |
There was a problem hiding this comment.
Not sure if this is needed.. @fjl ?
|
@holiman @fjl take another look please, made Console.clearHistory return an error and removed the text out confirmation. Another option would be to have clearHistory return |
|
Hm.. I was just about to press 'approve' on this, but then I decided to test it, and found two snags... When running geth as I do, This is a problem with the current code, not a new problem in this PR. To fix this, we really should have the ability to run Another problem is if you call the |
|
|
No description provided.