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
503

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?

522

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

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
503

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

522

Reinstated.

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