Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

haskell-process-get-repl-completions result contains unnecessary item #776

Closed
geraldus opened this issue Jul 24, 2015 · 5 comments
Closed

Comments

@geraldus
Copy link
Contributor

The result of GHCi REPL :complete repl command always outputs results summary in first line, it is consists of number of showed completions, number of all possible completions and of unused part of string for which completions were searched. E.g.

Prelude> :complete repl 2 "map co"
2 6 "map "
"compare"
"concat"

In the example above 2 is a number of returned completions, 6 is a number of all completions ("co" could be completed with 6 unique completions) and "map " is unused part of string (which was ignored, completions were searched only for "co" part). In case when completion requested for single word, for example :complete repl "ma", an empty string is returned as unused part always.

Currently, haskell-process-get-repl-completions1 returns mentioned unused part of input string as first element in resulting list:

(defun haskell-process-get-repl-completions (process inputstr)
  "Perform `:complete repl ...' query for INPUTSTR using PROCESS."
  (let* ((reqstr (concat ":complete repl "
                         (haskell-string-literal-encode inputstr)))
         (rawstr (haskell-process-queue-sync-request process reqstr)))
    (if (string-prefix-p "unknown command " rawstr)
        (error "GHCi lacks `:complete' support (try installing 7.8 or ghci-ng)")
      (let* ((s1 (split-string rawstr "\r?\n" t))
             (cs (mapcar #'haskell-string-literal-decode (cdr s1)))
             (h0 (car s1))) ;; "<cnt1> <cnt2> <quoted-str>"
        (unless (string-match "\\`\\([0-9]+\\) \\([0-9]+\\) \\(\".*\"\\)\\'" h0)
          (error "Invalid `:complete' response"))
        (let ((cnt1 (match-string 1 h0))
              (h1 (haskell-string-literal-decode (match-string 3 h0))))
          (unless (= (string-to-number cnt1) (length cs))
            (error "Lengths inconsistent in `:complete' reponse"))
          (cons h1 cs))))))

It stores this unused part in h1 variable. First, REPL response is being split by newlines into a list and stored in s1. Then it decodes all completions from s1's tail using haskell-string-literal-decode function (e.g. converts "smth" to lisp string object smth) and stores this in cs variable. The first line of response (init of s1) is stored in h0 variable and the last part of it (that is, unused part of input string) is stored in h1 variable using regular expression and string-match function. Finally, if all checks are passed it returns a cons cell of h1 and cs, as you can see in source code. So the first item of resulting list is that unused part of input.

Firstly, I think this is at least inconvenient with the function name, i.e. unused part of input is not a completion itself, while user may expect that all items in resulting list should be a valid completions for given input. Secondly, if there is no completions available the result should be empty list, while currently it always consists of one element at least. Thirdly, I don't know any use case of this information.

So, I want to fix this by not including unused part of input in results list. Any thoughts?

@gracjan
Copy link
Contributor

gracjan commented Jul 26, 2015

@geraldus: Can you reformulate this as a list of todo items (in github use - [ ] description) and then we will mark it as a well-defined-task.

@geraldus
Copy link
Contributor Author

Surely, but I want to make this change myself, because in my PR #772 I have already introduced a workaround for this in completion function (haskell-completions-sync-complete-repl). So I want to remove it and make changes to haskell-process-get-repl-completions directly as separate PR before #772 will be merged.

@gracjan
Copy link
Contributor

gracjan commented Jul 26, 2015 via email

@geraldus
Copy link
Contributor Author

@gracjan this function is synchronous (the command is executed using haskell-process-queue-sync-request), so I think it's okay to use error.

@geraldus
Copy link
Contributor Author

Done in #781

@gracjan gracjan closed this as completed Jul 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants