[PATCH] Introduce line editor library.

Peter Collingbourne peter at pcc.me.uk
Thu Jan 30 14:24:13 PST 2014



================
Comment at: lib/LineEditor/LineEditor.cpp:79
@@ +78,3 @@
+
+#ifdef HAVE_LIBEDIT
+
----------------
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.

================
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);
----------------
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.

================
Comment at: lib/LineEditor/LineEditor.cpp:43-53
@@ +42,13 @@
+
+  std::string CommonPrefix = Comps[0].TypedText;
+  for (std::vector<Completion>::iterator I = Comps.begin() + 1, E = Comps.end();
+       I != E; ++I) {
+    size_t Len = std::min(CommonPrefix.size(), I->TypedText.size());
+    size_t CommonLen = 0;
+    for (; CommonLen != Len; ++CommonLen) {
+      if (CommonPrefix[CommonLen] != I->TypedText[CommonLen])
+        break;
+    }
+    CommonPrefix.resize(CommonLen);
+  }
+
----------------
Manuel Klimek wrote:
> I'd pull that out into a method.
Done.

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


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



More information about the llvm-commits mailing list