[PATCH] D102402: LineEditor: Add a bare-bones readline-based implementation

[Hans Wennborg via Phabricator via llvm-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

Are the presubmit errors related to this?

The change itself lgtm



================
Comment at: llvm/lib/LineEditor/LineEditor.cpp:43
+  } while (Line.empty() ||
+           (Line[Line.size() - 1] != '\n' && Line[Line.size() - 1] != '\r'));
+
----------------
Maybe use Line.back() instead of Line[Line.size() - 1] here and below.


================
Comment at: llvm/lib/LineEditor/LineEditor.cpp:47
+         (Line[Line.size() - 1] == '\n' || Line[Line.size() - 1] == '\r'))
+    Line.resize(Line.size() - 1);
+
----------------
Could be pop_back() instead, which would be a nice match if using Line.back() == .. for the condition above.


================
Comment at: llvm/lib/LineEditor/LineEditor.cpp:30
+                                          FILE *Out) {
+  ::fprintf(Out, "%s", Prompt.c_str());
+
----------------
amccarth wrote:
> Nit:  In theory, this should be followed with a flush of Out.
> 
> In practice, it won't matter in most cases because stdio will ensure flushing stdout before accepting input on stdin.  But there's no guarantee here that In and Out correspond to the standard streams.  And since the prompt probably doesn't end with `'\n'`, you cannot count on that flushing either.
+1 I'd flush here just to be sure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102402/new/

https://reviews.llvm.org/D102402