[PATCH] Introduce line editor library.

[Manuel Klimek]

  lg


================
Comment at: lib/LineEditor/LineEditor.cpp:36
@@ +35,3 @@
+LineEditor::ListCompleterConcept::complete(StringRef Buffer, size_t Pos) const {
+  CompletionAction CA;
+  std::vector<Completion> Comps = getCompletions(Buffer, Pos);
----------------
Peter Collingbourne wrote:
> Manuel Klimek wrote:
> > Perhaps "Action"? Completion seems to be clear from the context.
> Well, you could have other actions associated with a line editor, like the action when a bound key is pressed or when a line is entered. So I think this name makes things clear.
Oh, I didn't mean the class name, I mean the local variable name (basically s/CA/Action/). I think "Action" is in this context easier to understand than CA. I strongly oppose initializations unless they are very very commonly used (an example is SM used in clang; but even there it quickly becomes a problem if it's not consistent, which it is not ;)

================
Comment at: lib/LineEditor/LineEditor.cpp:55
@@ +54,3 @@
+
+  if (CommonPrefix.empty()) {
+    CA.Kind = CompletionAction::AK_ShowCompletions;
----------------
Peter Collingbourne wrote:
> Manuel Klimek wrote:
> > That needs an explanation - why is the action kind "ShowCompletions" if we have no common prefix, and "Insert" otherwise?
> Added.
Ah, now it all makes sense - thanks!

================
Comment at: lib/LineEditor/LineEditor.cpp:79
@@ +78,3 @@
+
+#ifdef HAVE_LIBEDIT
+
----------------
Peter Collingbourne wrote:
> Manuel Klimek wrote:
> > I'd propose to have an interface, have two .cc files where the libedit one is only compiled if HAVE_LIBEDIT, and allow the main function of a tool to select one and inject it into the edit flow (while the tool that does the edit flow is free to have a default instantiation based on what is available).
> So this would allow clients to select the fgets implementation even if libedit is available, right? Is there any use case for such behaviour?
> 
> Also, in the long term I think we should have our own line editor implementation here, at which point there will be only one implementation.
To me having an interface would be simpler here - I agree that's a trade-off though, and I don't think it needs to block the patch from landing.


http://llvm-reviews.chandlerc.com/D2200

BRANCH
  clangquery2

ARCANIST PROJECT
  llvm