This is an archive of the discontinued LLVM Phabricator instance.

[llgo] cmd/llgoi: use line editor
ClosedPublic

Authored by axw on May 17 2015, 12:10 AM.

Details

Summary

This diff adds line-editing to llgoi, by
vendoring and using github.com/peterh/liner.
I have only implemented the basics here;
follow-ups will come later to add persisted
history, and completion,

Diff Detail

Event Timeline

axw updated this revision to Diff 25924.May 17 2015, 12:10 AM
axw retitled this revision from to [llgo] cmd/llgoi: use line editor.
axw updated this object.
axw edited the test plan for this revision. (Show Details)
axw added a reviewer: pcc.
axw added a subscriber: Unknown Object (MLST).
pcc edited edge metadata.May 17 2015, 4:46 PM

Thanks for working on this much-needed usability improvement.

I played around with your change locally, and it seems to break one of the tests (llgoi/panic.test).

cmd/llgoi/llgoi.go
522

It looks like liner doesn't do this for us, so we probably need to keep doing it ourselves.

539

It doesn't look like liner deals with multi-line history entries very well. (e.g. try typing "{<nl>}<nl><up>" and then editing the line). Can we maybe strip newlines and apply semicolon insertion where necessary instead of saving the literal newlines?

axw added a comment.May 19 2015, 6:11 AM
In D9811#174002, @pcc wrote:

Thanks for working on this much-needed usability improvement.

I played around with your change locally, and it seems to break one of the tests (llgoi/panic.test).

Fixed. Prompt() doesn't return the newline, so we need to add one.

axw updated this revision to Diff 26053.May 19 2015, 6:17 AM
axw edited edge metadata.

Collapse history lines, add semicolons

Also, append newlines to lines passed
to readLine, and reinstate newline
written when in terminal mode.

axw added a comment.May 21 2015, 10:14 PM

Sorry, just realised I hadn't submitted my replies. PTAL.

cmd/llgoi/llgoi.go
522

Reinstated.

539

I've done that now. Lines are collapsed, and semicolons (added by go/scanner) are written where necessary.

pcc accepted this revision.May 22 2015, 11:59 AM
pcc edited edge metadata.

Please add liner to the list of third-party packages in LICENSE.TXT. Other than that, LGTM.

update_third_party.sh
104

Wouldn't this leave a .git directory in third_party/liner?

This revision is now accepted and ready to land.May 22 2015, 11:59 AM
axw updated this revision to Diff 26370.May 23 2015, 8:18 AM
axw edited edge metadata.

Fix update_third_party.sh, update LICENSE.TXT

axw added a comment.May 23 2015, 8:19 AM

Thanks, updated LICENSE.TXT.

update_third_party.sh
104

Yes, added && rm -fr .git.

axw closed this revision.May 23 2015, 8:19 AM
./CMakeLists.txt