Skip to content
This repository has been archived by the owner on Jun 4, 2022. It is now read-only.

on empty line-suffix, don't get completions #333

Closed
wants to merge 2 commits into from

Conversation

hlolli
Copy link
Collaborator

@hlolli hlolli commented Dec 19, 2017

This is an old pain, but this is not a final solution as symbol names ending with alphanumeric number are legal symbols in Clojure as far as I can tell. Next step should be to modify (re-find #":?([a-zA-Z-.<>*=&?]*|^\(/)$" line) to something better (including number, or match all except xy). In any case tough, if this re-find doesn't find anything, (ie. invalid?) then completions should not continue, as empty string causes something like this

(time (do (lumo.repl/get-completions "red6" #(prn %)) nil))
d6qualified-ident?" "red6qualified-keyword?" "red6qualified-symbol?" "red6quot" "red6rand" "red6rand-int" "red6rand-nth" "red6random-sample" "red6random-uuid" "red6range" "red6ranged-iterator" "red6re-find" "red6re-matches" "red6re-pattern" "red6re-seq" "red6realized?" "red6record?" "red6reduce" "red6reduce-kv" "red6reduceable?" "red6reduced" "red6reduced?" "red6reductions" "red6refer-clojure" "red6regexp?" "red6reify" "red6rem" "red6remove" "red6remove-all-methods" "red6remove-method" "red6remove-watch" "red6repeat" "red6repeatedly" "red6replace" "red6replicate" "red6require" "red6require-macros" "red6reset!" "red6reset-meta!" "red6reset-vals!" "red6resolve" "red6rest" "red6reverse" "red6reversible?" "red6rseq" "red6rsubseq" "red6run!" "red6satisfies?" "red6second" "red6select-keys" "red6seq" "red6seq-iter" "red6seq?" "red6seqable?" "red6sequence" "red6sequential?" "red6set" "red6set-from-indexed-seq" "red6set-print-err-fn!" "red6set-print-fn!" "red6set-validator!" "red6set?" "red6short" "red6shorts" "red6shuffle" "red6simple-benchmark" "red6simple-ident?" "red6simple-keyword?" "red6simple-symbol?" "red6some" "red6some->" "red6some->>" "red6some-fn" "red6some?" "red6someprotopackage.TestPackageTypes" "red6sort" "red6sort-by" "red6sorted-map" "red6sorted-map-by" "red6sorted-set" "red6sorted-set-by" "red6sorted?" "red6source" "red6special-symbol?" "red6specify" "red6specify!" "red6split-at" "red6split-with" "red6spread" "red6str" "red6string-hash-cache" 
..elided...
"Elapsed time: 553.308537 msecs"

@anmonteiro
Copy link
Owner

clearly tests are failing

@hlolli
Copy link
Collaborator Author

hlolli commented Dec 19, 2017

node target/bundle.js -c target/ -n ""

Lumo 1.8.0-beta
ClojureScript 1.9.946
Node.js v9.2.0
 Docs: (doc function-name-here)
       (find-doc "part-of-name-here")
 Source: (source function-name-here)
 Exit: Control+D or :cljs/quit or exit

Error: Port should be > 0 and < 65536. Received NaN.
cljs.user=> 

Same output for latest release and the dev build, the test is expecting

"Error: "port" argument must be >= 0 and < 65536

I'm clueless how this and my last commit could have caused this change. Different node version?

@hlolli
Copy link
Collaborator Author

hlolli commented Dec 19, 2017

Should I edit the snapshot, or modify the error message?

@arichiardi
Copy link
Collaborator

Was it the node version the problem?

@arichiardi
Copy link
Collaborator

Node version might have changed the message, see #334

@arichiardi
Copy link
Collaborator

@hlolli I fixed the problems with CI, I was wondering if you can rebase and retry

completions (reduce (fn [ret item]
(doto ret
(.push (str line-prefix item))))
completions (if (empty? line-match-suffix)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised that the completion-candidates fn does not depend on the prefix item. I would expect it is it's responsibility to do the if. Let me try to push a commit here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see completion-candidates comes up with all the completions all the time

@arichiardi
Copy link
Collaborator

I took a little bit of time to check what is going on here and opened an alternative branch. Hopefully the fixes are good enough. We cannot filter out everything when line-match-suffix is empty because we would also kill legit completions for namespaces/built ins/goog.

arichiardi added a commit that referenced this pull request Jan 15, 2018
The new tests make sure lumo correctly handles completions for symbols with
numbers (see #333), lines starting with (, ending with / or completely empty.
arichiardi added a commit that referenced this pull request Jan 16, 2018
The new tests make sure lumo correctly handles completions for symbols with
numbers (see #333), lines starting with (, ending with / or completely empty.
arichiardi added a commit that referenced this pull request Jan 18, 2018
The new tests make sure lumo correctly handles completions for symbols with
numbers (see #333), lines starting with (, ending with / or completely empty.
@anmonteiro
Copy link
Owner

I'm going to close this in favor of #337

@anmonteiro anmonteiro closed this Jan 23, 2018
arichiardi added a commit that referenced this pull request Jan 25, 2018
The new tests make sure lumo correctly handles completions for symbols with
numbers (see #333), lines starting with (, ending with / or completely empty.
arichiardi added a commit that referenced this pull request Jan 25, 2018
The new tests make sure lumo correctly handles completions for symbols with
numbers (see #333), lines starting with (, ending with / or completely empty.
arichiardi added a commit that referenced this pull request Jan 26, 2018
The new tests make sure lumo correctly handles completions for symbols with
numbers (see #333), lines starting with (, ending with / or completely empty.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants