LibLine: Unify completion hooks and adapt its users

LibLine should ultimately not care about what a "token" means in the
context of its user, so force the user to split the buffer itself.
This also allows the users to pick up contextual clues as well, since
they have to lex the line themselves.

This commit pacthes Shell and the JS repl to better handle completions,
so certain wrong behaviours are now corrected as well:
- JS repl can now complete "Object . getOw<tab>"
- Shell can now complete "echo | ca<tab>" and paths inside strings

Libraries/LibLine/Editor.cpp

@@ -38,7 +38,7 @@
 namespace Line {
 
 Editor::Editor(Configuration configuration)
-    : m_configuration(configuration)
+    : m_configuration(move(configuration))
 {
     m_always_refresh = configuration.refresh_behaviour == Configuration::RefreshBehaviour::Eager;
     m_pending_chars = ByteBuffer::create_uninitialized(0);
@@ -430,54 +430,17 @@ String Editor::get_line(const String& prompt)
             m_search_offset = 0; // reset search offset on any key
 
             if (codepoint == '\t' || reverse_tab) {
-                if (!on_tab_complete_first_token || !on_tab_complete_other_token)
+                if (!on_tab_complete)
                     continue;
 
-                auto should_break_token = [mode = m_configuration.split_mechanism](auto& buffer, size_t index) {
-                    switch (mode) {
-                    case Configuration::TokenSplitMechanism::Spaces:
-                        return buffer[index] == ' ';
-                    case Configuration::TokenSplitMechanism::UnescapedSpaces:
-                        return buffer[index] == ' ' && (index == 0 || buffer[index - 1] != '\\');
-                    }
-
-                    ASSERT_NOT_REACHED();
-                    return true;
-                };
-
-                bool is_empty_token = m_cursor == 0 || should_break_token(m_buffer, m_cursor - 1);
-
                 // reverse tab can count as regular tab here
                 m_times_tab_pressed++;
 
-                int token_start = m_cursor - 1;
-
-                if (!is_empty_token) {
-                    while (token_start >= 0 && !should_break_token(m_buffer, token_start))
-                        --token_start;
-                    ++token_start;
-                }
-
-                bool is_first_token = true;
-                for (int i = token_start - 1; i >= 0; --i) {
-                    if (should_break_token(m_buffer, i)) {
-                        is_first_token = false;
-                        break;
-                    }
-                }
-
-                StringBuilder builder;
-                builder.append(Utf32View { m_buffer.data() + token_start, m_cursor - token_start });
-                String token = is_empty_token ? String() : builder.to_string();
-
                 // ask for completions only on the first tab
                 // and scan for the largest common prefix to display
                 // further tabs simply show the cached completions
                 if (m_times_tab_pressed == 1) {
-                    if (is_first_token)
+                    m_suggestions = on_tab_complete(*this);
-                        m_suggestions = on_tab_complete_first_token(token);
-                    else
-                        m_suggestions = on_tab_complete_other_token(token);
                     size_t common_suggestion_prefix { 0 };
                     if (m_suggestions.size() == 1) {
                         m_largest_common_suggestion_prefix_length = m_suggestions[0].text.length();
@@ -1267,11 +1230,24 @@ Vector<size_t, 2> Editor::vt_dsr()
     return { x, y };
 }
 
-String Editor::line() const
+String Editor::line(size_t up_to_index) const
 {
     StringBuilder builder;
-    builder.append(Utf32View { m_buffer.data(), m_buffer.size() });
+    builder.append(Utf32View { m_buffer.data(), min(m_buffer.size(), up_to_index) });
     return builder.build();
 }
 
+bool Editor::should_break_token(Vector<u32, 1024>& buffer, size_t index)
+{
+    switch (m_configuration.split_mechanism) {
+    case Configuration::TokenSplitMechanism::Spaces:
+        return buffer[index] == ' ';
+    case Configuration::TokenSplitMechanism::UnescapedSpaces:
+        return buffer[index] == ' ' && (index == 0 || buffer[index - 1] != '\\');
+    }
+
+    ASSERT_NOT_REACHED();
+    return true;
+};
+
 }

Libraries/LibLine/Editor.h

@@ -45,14 +45,6 @@ namespace Line {
 
 class Editor;
 
-struct KeyCallback {
-    KeyCallback(Function<bool(Editor&)> cb)
-        : callback(move(cb))
-    {
-    }
-    Function<bool(Editor&)> callback;
-};
-
 struct CompletionSuggestion {
     // intentionally not explicit (allows suggesting bare strings)
     CompletionSuggestion(const String& completion)
@@ -132,8 +124,7 @@ public:
     void register_character_input_callback(char ch, Function<bool(Editor&)> callback);
     size_t actual_rendered_string_length(const StringView& string) const;
 
-    Function<Vector<CompletionSuggestion>(const String&)> on_tab_complete_first_token;
+    Function<Vector<CompletionSuggestion>(const Editor&)> on_tab_complete;
-    Function<Vector<CompletionSuggestion>(const String&)> on_tab_complete_other_token;
     Function<void()> on_interrupt_handled;
     Function<void(Editor&)> on_display_refresh;
 
@@ -149,7 +140,8 @@ public:
     size_t cursor() const { return m_cursor; }
     const Vector<u32, 1024>& buffer() const { return m_buffer; }
     u32 buffer_at(size_t pos) const { return m_buffer.at(pos); }
-    String line() const;
+    String line() const { return line(m_buffer.size()); }
+    String line(size_t up_to_index) const;
 
     // only makes sense inside a char_input callback or on_* callback
     void set_prompt(const String& prompt)
@@ -171,7 +163,7 @@ public:
         m_spans_ending.clear();
         m_refresh_needed = true;
     }
-    void suggest(size_t invariant_offset = 0, size_t index = 0)
+    void suggest(size_t invariant_offset = 0, size_t index = 0) const
     {
         m_next_suggestion_index = index;
         m_next_suggestion_invariant_offset = invariant_offset;
@@ -188,6 +180,14 @@ public:
     bool is_editing() const { return m_is_editing; }
 
 private:
+    struct KeyCallback {
+        KeyCallback(Function<bool(Editor&)> cb)
+            : callback(move(cb))
+        {
+        }
+        Function<bool(Editor&)> callback;
+    };
+
     void vt_save_cursor();
     void vt_restore_cursor();
     void vt_clear_to_end_of_line();
@@ -265,6 +265,9 @@ private:
         m_origin_x = position[0];
         m_origin_y = position[1];
     }
+
+    bool should_break_token(Vector<u32, 1024>& buffer, size_t index);
+
     void recalculate_origin();
     void reposition_cursor();
 
@@ -304,8 +307,8 @@ private:
     CompletionSuggestion m_last_shown_suggestion { String::empty() };
     size_t m_last_shown_suggestion_display_length { 0 };
     bool m_last_shown_suggestion_was_complete { false };
-    size_t m_next_suggestion_index { 0 };
+    mutable size_t m_next_suggestion_index { 0 };
-    size_t m_next_suggestion_invariant_offset { 0 };
+    mutable size_t m_next_suggestion_invariant_offset { 0 };
     size_t m_largest_common_suggestion_prefix_length { 0 };
     size_t m_last_displayed_suggestion_index { 0 };
 

Shell/Shell.cpp

@@ -1470,102 +1470,76 @@ void Shell::highlight(Line::Editor&) const
     }
 }
 
-Vector<Line::CompletionSuggestion> Shell::complete_first(const String& token_to_complete)
+Vector<Line::CompletionSuggestion> Shell::complete(const Line::Editor& editor)
 {
-    auto token = unescape_token(token_to_complete);
+    auto line = editor.line(editor.cursor());
 
-    auto match = binary_search(cached_path.data(), cached_path.size(), token, [](const String& token, const String& program) -> int {
+    Parser parser(line);
-        return strncmp(token.characters(), program.characters(), token.length());
-    });
 
-    if (!match) {
+    auto commands = parser.parse();
-        // There is no executable in the $PATH starting with $token
-        // Suggest local executables and directories
-        String path;
-        Vector<Line::CompletionSuggestion> local_suggestions;
-        bool suggest_executables = true;
 
-        ssize_t last_slash = token.length() - 1;
+    if (commands.size() == 0)
-        while (last_slash >= 0 && token[last_slash] != '/')
+        return {};
-            --last_slash;
 
-        if (last_slash >= 0) {
+    // get the last token and whether it's the first in its subcommand
-            // Split on the last slash. We'll use the first part as the directory
+    String token;
-            // to search and the second part as the token to complete.
+    bool is_first_in_subcommand = false;
-            path = token.substring(0, last_slash + 1);
+    auto& subcommand = commands.last().subcommands;
-            if (path[0] != '/')
+
-                path = String::format("%s/%s", cwd.characters(), path.characters());
+    if (subcommand.size() == 0) {
-            path = canonicalized_path(path);
+        // foo bar; <tab>
-            token = token.substring(last_slash + 1, token.length() - last_slash - 1);
+        token = "";
+        is_first_in_subcommand = true;
+    } else {
+        auto& last_command = subcommand.last();
+        if (last_command.args.size() == 0) {
+            // foo bar | <tab>
+            token = "";
+            is_first_in_subcommand = true;
         } else {
-            // We have no slashes, so the directory to search is the current
+            auto& args = last_command.args;
-            // directory and the token to complete is just the original token.
+            if (args.last().type == Token::Comment) // we cannot complete comments
-            // In this case, do not suggest executables but directories only.
+                return {};
-            path = cwd;
+
-            suggest_executables = false;
+            is_first_in_subcommand = args.size() == 1;
+            token = last_command.args.last().text;
         }
+    }
 
-        // the invariant part of the token is actually just the last segment
+    Vector<Line::CompletionSuggestion> suggestions;
-        // e. in `cd /foo/bar', 'bar' is the invariant
-        //      since we are not suggesting anything starting with
-        //      `/foo/', but rather just `bar...'
-        editor.suggest(escape_token(token).length(), 0);
 
-        // only suggest dot-files if path starts with a dot
+    bool should_suggest_only_executables = false;
-        Core::DirIterator files(path,
-            token.starts_with('.') ? Core::DirIterator::SkipParentAndBaseDir : Core::DirIterator::SkipDots);
 
-        while (files.has_next()) {
+    if (is_first_in_subcommand) {
-            auto file = files.next_path();
+        auto match = binary_search(cached_path.data(), cached_path.size(), token, [](const String& token, const String& program) -> int {
-            auto trivia = " ";
+            return strncmp(token.characters(), program.characters(), token.length());
-            if (file.starts_with(token)) {
+        });
-                String file_path = String::format("%s/%s", path.characters(), file.characters());
-                struct stat program_status;
-                int stat_error = stat(file_path.characters(), &program_status);
-                if (stat_error)
-                    continue;
-                if (access(file_path.characters(), X_OK) != 0)
-                    continue;
-                if (S_ISDIR(program_status.st_mode)) {
-                    if (!suggest_executables)
-                        continue;
-                    else
-                        trivia = "/";
-                }
 
-                local_suggestions.append({ escape_token(file), trivia });
+        if (match) {
+            String completion = *match;
+            editor.suggest(escape_token(token).length(), 0);
+
+            // Now that we have a program name starting with our token, we look at
+            // other program names starting with our token and cut off any mismatching
+            // characters.
+
+            int index = match - cached_path.data();
+            for (int i = index - 1; i >= 0 && cached_path[i].starts_with(token); --i) {
+                suggestions.append({ cached_path[i], " " });
             }
+            for (size_t i = index + 1; i < cached_path.size() && cached_path[i].starts_with(token); ++i) {
+                suggestions.append({ cached_path[i], " " });
+            }
+            suggestions.append({ cached_path[index], " " });
+
+            return suggestions;
         }
 
-        return local_suggestions;
+        // fallthrough to suggesting local files, but make sure to only suggest executables
+        should_suggest_only_executables = true;
     }
 
-    String completion = *match;
-    Vector<Line::CompletionSuggestion> suggestions;
-
-    // Now that we have a program name starting with our token, we look at
-    // other program names starting with our token and cut off any mismatching
-    // characters.
-
-    int index = match - cached_path.data();
-    for (int i = index - 1; i >= 0 && cached_path[i].starts_with(token); --i) {
-        suggestions.append({ cached_path[i], " " });
-    }
-    for (size_t i = index + 1; i < cached_path.size() && cached_path[i].starts_with(token); ++i) {
-        suggestions.append({ cached_path[i], " " });
-    }
-    suggestions.append({ cached_path[index], " " });
-
-    editor.suggest(escape_token(token).length(), 0);
-
-    return suggestions;
-}
-
-Vector<Line::CompletionSuggestion> Shell::complete_other(const String& token_to_complete)
-{
-    auto token = unescape_token(token_to_complete);
     String path;
-    Vector<Line::CompletionSuggestion> suggestions;
 
     ssize_t last_slash = token.length() - 1;
     while (last_slash >= 0 && token[last_slash] != '/')
@@ -1602,10 +1576,12 @@ Vector<Line::CompletionSuggestion> Shell::complete_other(const String& token_to_
             String file_path = String::format("%s/%s", path.characters(), file.characters());
             int stat_error = stat(file_path.characters(), &program_status);
             if (!stat_error) {
-                if (S_ISDIR(program_status.st_mode))
+                if (S_ISDIR(program_status.st_mode)) {
-                    suggestions.append({ escape_token(file), "/" });
+                    if (!should_suggest_only_executables)
-                else
+                        suggestions.append({ escape_token(file), "/" });
+                } else {
                     suggestions.append({ escape_token(file), " " });
+                }
             }
         }
     }

Shell/Shell.h

@@ -112,8 +112,7 @@ public:
     static ContinuationRequest is_complete(const Vector<Command>&);
 
     void highlight(Line::Editor&) const;
-    Vector<Line::CompletionSuggestion> complete_first(const String&);
+    Vector<Line::CompletionSuggestion> complete(const Line::Editor&);
-    Vector<Line::CompletionSuggestion> complete_other(const String&);
 
     String get_history_path();
     void load_history();

Shell/main.cpp

@@ -75,11 +75,8 @@ int main(int argc, char** argv)
         editor.strip_styles();
         shell->highlight(editor);
     };
-    editor.on_tab_complete_first_token = [&](const String& token_to_complete) -> Vector<Line::CompletionSuggestion> {
+    editor.on_tab_complete = [&](const Line::Editor& editor) {
-        return shell->complete_first(token_to_complete);
+        return shell->complete(editor);
-    };
-    editor.on_tab_complete_other_token = [&](const String& token_to_complete) -> Vector<Line::CompletionSuggestion> {
-        return shell->complete_other(token_to_complete);
     };
 
     signal(SIGINT, [](int) {

Userland/js.cpp

@@ -655,17 +655,75 @@ int main(int argc, char** argv)
             editor.set_prompt(prompt_for_level(open_indents));
         };
 
-        auto complete = [&interpreter, &editor = *s_editor](const String& token) -> Vector<Line::CompletionSuggestion> {
+        auto complete = [&interpreter](const Line::Editor& editor) -> Vector<Line::CompletionSuggestion> {
-            if (token.length() == 0)
+            auto line = editor.line(editor.cursor());
-                return {}; // nyeh
+
+            JS::Lexer lexer { line };
+            enum {
+                Initial,
+                CompleteVariable,
+                CompleteNullProperty,
+                CompleteProperty,
+            } mode { Initial };
+
+            StringView variable_name;
+            StringView property_name;
 
-            auto line = editor.line();
             // we're only going to complete either
             //    - <N>
             //        where N is part of the name of a variable
             //    - <N>.<P>
             //        where N is the complete name of a variable and
             //        P is part of the name of one of its properties
+            auto js_token = lexer.next();
+            for (; js_token.type() != JS::TokenType::Eof; js_token = lexer.next()) {
+                switch (mode) {
+                case CompleteVariable:
+                    switch (js_token.type()) {
+                    case JS::TokenType::Period:
+                        // ...<name> <dot>
+                        mode = CompleteNullProperty;
+                        break;
+                    default:
+                        // not a dot, reset back to initial
+                        mode = Initial;
+                        break;
+                    }
+                    break;
+                case CompleteNullProperty:
+                    if (js_token.is_identifier_name()) {
+                        // ...<name> <dot> <name>
+                        mode = CompleteProperty;
+                        property_name = js_token.value();
+                    } else {
+                        mode = Initial;
+                    }
+                    break;
+                case CompleteProperty:
+                    // something came after the property access, reset to initial
+                case Initial:
+                    if (js_token.is_identifier_name()) {
+                        // ...<name>...
+                        mode = CompleteVariable;
+                        variable_name = js_token.value();
+                    } else {
+                        mode = Initial;
+                    }
+                    break;
+                }
+            }
+
+            bool last_token_has_trivia = js_token.trivia().length() > 0;
+
+            if (mode == CompleteNullProperty) {
+                mode = CompleteProperty;
+                property_name = "";
+                last_token_has_trivia = false; // <name> <dot> [tab] is sensible to complete.
+            }
+
+            if (mode == Initial || last_token_has_trivia)
+                return {}; // we do not know how to complete this
+
             Vector<Line::CompletionSuggestion> results;
 
             Function<void(const JS::Shape&, const StringView&)> list_all_properties = [&results, &list_all_properties](const JS::Shape& shape, auto& property_pattern) {
@@ -682,41 +740,40 @@ int main(int argc, char** argv)
                 }
             };
 
-            if (token.contains(".")) {
+            switch (mode) {
-                auto parts = token.split('.', true);
+            case CompleteProperty: {
-                // refuse either `.` or `a.b.c`
+                auto maybe_variable = interpreter->get_variable(variable_name);
-                if (parts.size() > 2 || parts.size() == 0)
-                    return {};
-
-                auto name = parts[0];
-                auto property_pattern = parts[1];
-
-                auto maybe_variable = interpreter->get_variable(name);
                 if (maybe_variable.is_empty()) {
-                    maybe_variable = interpreter->global_object().get(name);
+                    maybe_variable = interpreter->global_object().get(variable_name);
                     if (maybe_variable.is_empty())
-                        return {};
+                        break;
                 }
 
                 auto variable = maybe_variable;
                 if (!variable.is_object())
-                    return {};
+                    break;
 
                 const auto* object = variable.to_object(*interpreter);
                 const auto& shape = object->shape();
-                list_all_properties(shape, property_pattern);
+                list_all_properties(shape, property_name);
                 if (results.size())
-                    editor.suggest(property_pattern.length());
+                    editor.suggest(property_name.length());
-                return results;
+                break;
             }
-            const auto& variable = interpreter->global_object();
+            case CompleteVariable: {
-            list_all_properties(variable.shape(), token);
+                const auto& variable = interpreter->global_object();
-            if (results.size())
+                list_all_properties(variable.shape(), variable_name);
-                editor.suggest(token.length());
+                if (results.size())
+                    editor.suggest(variable_name.length());
+                break;
+            }
+            default:
+                ASSERT_NOT_REACHED();
+            }
+
             return results;
         };
-        s_editor->on_tab_complete_first_token = [complete](auto& value) { return complete(value); };
+        s_editor->on_tab_complete = move(complete);
-        s_editor->on_tab_complete_other_token = [complete](auto& value) { return complete(value); };
         repl(*interpreter);
     } else {
         interpreter = JS::Interpreter::create<JS::GlobalObject>();