alexander-yakushev / compliment

Clojure completion library that you deserve
Eclipse Public License 1.0
355 stars 38 forks source link

Workaround to avoid to replace '\{' and '\}' to '\(compliment-hashmap' and '\)' respectively #87

Closed vmfhrmfoaj closed 2 years ago

vmfhrmfoaj commented 2 years ago

In try-read-replacing-maps function, in order to read a context, replace '{' and '}' with '(compliment-hashmap' and ')' respectively without condition. '\{' (literal '{') is also replaced with '\(compliment-hashmap'. (read-string cannot parse a string containing '\('.)

So if a context contains '{' or '}', can't extract variable names from the context due to safe-read-context-string return nil. To avoid the problem, I fixed try-read-replacing-maps to replace '\{' and '\}' with '(char 123)' and '(char 125)' respectively before replacing '{' and '}'.

;; before changes:
(criterium.core/quick-bench (try-read-replacing-maps "(defn glob->regex
  \"Convert a glob pattern string to a regular expression.
  I additionally support '**' pattern. This pattern indicates all characters except the newline.

  Examples:
   (glob->regex \\\"/a/b/*\\\")       ;=> #\\\"/a/b/[^/]*\\\"
   (glob->regex \\\"/a/b/?\\\")       ;=> #\\\"/a/b/[^/]?\\\"
   (glob->regex \\\"/a/b/[abc]\\\")   ;=> #\\\"/a/b/[abc]\\\"
   (glob->regex \\\"/a/b/[a-c]\\\")   ;=> #\\\"/a/b/[a-c]\\\"
   (glob->regex \\\"/a/b/{a,b,c}\\\") ;=> #\\\"/a/b/(a|b|c)\\\"
   (glob->regex \\\"/a/b/\\\\*\\\")     ;=> #\\\"/a/b/*\\\"\"
  [input]
  (loop [[c & cs] input
         regex \"\"
         escaped? false
         in-bracket? false
         in-curly-bracket? false]
    __prefix__
    (= c \\{) nil
    (= c \\}) nil
    ))
"))
;; Evaluation count : 19890 in 6 samples of 3315 calls.
;;              Execution time mean : 29.990710 µs
;;     Execution time std-deviation : 86.132061 ns
;;    Execution time lower quantile : 29.840915 µs ( 2.5%)
;;    Execution time upper quantile : 30.066819 µs (97.5%)
;;                    Overhead used : 5.268679 ns
;;
;; Found 1 outliers in 6 samples (16.6667 %)
;;  low-severe   1 (16.6667 %)
;;  Variance from outliers : 13.8889 % Variance is moderately inflated by outliers
nil

;; after changes:
(criterium.core/quick-bench (try-read-replacing-maps "(defn glob->regex ..."))
;; Evaluation count : 17676 in 6 samples of 2946 calls.
;;              Execution time mean : 34.312736 µs
;;     Execution time std-deviation : 189.613207 ns
;;    Execution time lower quantile : 33.954349 µs ( 2.5%)
;;    Execution time upper quantile : 34.449656 µs (97.5%)
;;                    Overhead used : 5.268679 ns
;;
;; Found 1 outliers in 6 samples (16.6667 %)
;;  low-severe   1 (16.6667 %)
;;  Variance from outliers : 13.8889 % Variance is moderately inflated by outliers
nil

;; regular expression version:
#_(-> s
      (str/replace #"(?<=^|[^\\])\{" "(compliment-hashmap ")
      (str/replace #"(?<=^|[^\\])\}" ")")
      ...)
(criterium.core/quick-bench (try-read-replacing-maps "(defn glob->regex ..."))
;; Evaluation count : 6054 in 6 samples of 1009 calls.
;;              Execution time mean : 101.963995 µs
;;     Execution time std-deviation : 258.471182 ns
;;    Execution time lower quantile : 101.689522 µs ( 2.5%)
;;    Execution time upper quantile : 102.275322 µs (97.5%)
;;                    Overhead used : 5.268679 ns
nil
vmfhrmfoaj commented 2 years ago

@bbatsov Updated. Thank you for the review.

bbatsov commented 2 years ago

I've cut 0.3.14 that includes your fix and hopefully addresses #86 as well.