Fix history command when extra commands are processed (#204)

* Fix history command when extra commands are processed

* Unparse next-cmd ast in pipe-cmds

* Fix test

* Use lein-bullseye container

.github/workflows/release.yml

@@ -12,8 +12,7 @@ jobs:
     # Containers must run in Linux based operating systems
     runs-on: ubuntu-latest
     # Docker Hub image that `container-job` executes in
-    container: clojure:lein
-
+    container: clojure:lein-bullseye
     services:
       # Label used to access the service container
       postgres:

.github/workflows/test.yml

@@ -11,7 +11,7 @@ jobs:
 
   test:
     runs-on: ubuntu-latest
-    container: clojure:lein
+    container: clojure:lein-bullseye
     services:
       postgres:
         image: postgres

project.clj

@@ -56,7 +56,7 @@
                     :yb-adapters-freenode-host "irc.freenode.net"
                     :yb-adapters-freenode-port "6667"
                     :yb-adapters-freenode-username "yetibot-test"}}}
-  :dependencies [[org.clojure/clojure "1.10.0"]
+  :dependencies [[org.clojure/clojure "1.10.3"]
                  [org.clojure/core.async "0.4.500"]
                  [org.clojure/data.json "0.2.6"]
                  [org.clojure/tools.cli "0.4.2"]

src/yetibot/core/adapters.clj

@@ -77,6 +77,7 @@
 
 (comment
   (start)
+  (stop)
 
   (a/active-adapters)
 

src/yetibot/core/commands/history.clj

@@ -3,7 +3,7 @@
    [yetibot.core.util.command :refer [config-prefix]]
    [yetibot.core.db.util :refer [merge-queries]]
    [clojure.tools.cli :refer [parse-opts]]
-   [taoensso.timbre :refer [debug info color-str]]
+   [taoensso.timbre :refer [debug info trace color-str]]
    [yetibot.core.models.history :as h]
    [yetibot.core.db.history :refer [query]]
    [clojure.string :refer [blank? join split]]
@@ -13,11 +13,38 @@
 
 (defn split-cmd [cmd-with-args] (split cmd-with-args #"\s"))
 
-(defn should-consume-cmd? [cmd-with-args]
+; (defn should-consume-cmd?
+;   "arg is the command ast, e.g. [:cmd [:words \"count\"]]"
+;   [[_ [_ cmd]]]
+;   (trace "should-consume-cmd?" (pr-str cmd))
+;   (consumables cmd))
+
+(defn should-consume-cmd?
+  "arg is the command nad its args, e.g.:
+   [\"count\"]
+   [\"head\" 2]"
+  [cmd-with-args]
+  (info (pr-str "should-consume-cmd?" cmd-with-args))
   (let [[cmd & _] (split-cmd cmd-with-args)]
     (consumables cmd)))
 
-(defn history-for-cmd-sequence [next-cmds extra-query]
+(defn history-for-cmd-sequence
+  "Given a command that follows `history`, e.g.:
+
+   `history | grep foo`
+
+   Look up the history for the correct command.
+
+   Note: in theory multiple commands following `history` could work, e.g.:
+
+   `history | grep foo | count`
+
+   but currently we only support a single command following `history`."
+  [next-cmds extra-query]
+  (debug "history-for-cmd-sequence"
+        (pr-str {:next-cmds next-cmds
+                 :extra-query extra-query}))
+  ;; only inspect the first `next-cmd`
   (let [[next-cmd & args] (split-cmd (first next-cmds))
         ;; only head and tail accept an integer arg
         possible-int-arg (or (when (and (not (empty? args))
@@ -126,8 +153,8 @@
    history | random - uses LIMIT 1 Postgres' ORDER_BY random()
    history | count - uses COUNT(*)"
   {:yb/cat #{:util}}
-  [{:keys [match chat-source next-cmds skip-next-n]}]
-  (info "history-cmd match" (pr-str match))
+  [{:keys [match chat-source next-cmds skip-next-n] :as cmd-map}]
+  (debug "history-cmd cmd-map" (pr-str cmd-map))
   ;; for now, only look at the first item from `next-cmds`. eventually we may
   ;; support some sort of query combinator that could calculate query for
   ;; multiple steps, like: history | head 30 | grep foo | count
@@ -144,8 +171,7 @@
       (let [;; figure out how many commands to consume
             skip-n (count (take-while should-consume-cmd? (take 1 next-cmds)))
 
-            ;; TODO remove after dev
-            _ (info "history options" (color-str :green (pr-str parsed)))
+            _ (trace "history options" (color-str :green (pr-str parsed)))
 
             ;; build up a vector of query maps using provided `options` we'll
             ;; merge these into a single map later
@@ -232,7 +258,6 @@
                 :where/args [(:until options)]}))
 
             ;; now merge the vector of maps into one single map
-
             _ (info "extra queries to merge" (pr-str extra-queries))
             extra-query (apply merge-queries extra-queries)
             _ (info "extra query" (pr-str extra-query))
@@ -259,3 +284,12 @@
 
 (cmd-hook #"history"
   _ history-cmd)
+
+(comment
+  (history-cmd {:match ""
+                :skip-next-n (atom nil)
+                :next-cmds [[:cmd [:words "grep" [:space " "] "minutes"]]]})
+
+  *e
+  )
+

src/yetibot/core/handler.clj

@@ -9,7 +9,8 @@
    [yetibot.core.util.command :refer [extract-command embedded-cmds]]
    [yetibot.core.interpreter :as interp]
    [yetibot.core.models.history :as h]
-   [yetibot.core.parser :refer [parse-and-eval transformer parser unparse]]
+   [yetibot.core.unparser :refer [unparse]]
+   [yetibot.core.parser :refer [parse-and-eval transformer parser]]
    [yetibot.core.util.format :refer [format-data-structure
                                      format-exception-log]]))
 

src/yetibot/core/interpreter.clj

@@ -2,6 +2,7 @@
   "Handles evaluation of a parse tree"
   (:require
     [yetibot.core.chat :refer [suppress]]
+    [yetibot.core.unparser :refer [unparse-transformer]]
     [yetibot.core.models.default-command :refer [fallback-enabled?
                                                  configured-default-command]]
     [yetibot.core.models.channel :as channel]
@@ -44,8 +45,8 @@
 (defn pipe-cmds
   "Pipe acc into cmd-with-args by either appending or sending acc as an extra
    :opts"
-  [evaluator acc [cmd-ast & next-cmds]]
-  (debug "pipe-cmds" *chat-source* acc cmd-ast next-cmds)
+  [evaluator acc [cmd-ast & next-cmds-ast]]
+  (debug "pipe-cmds" *chat-source* acc cmd-ast next-cmds-ast)
   (let [;; the previous accumulated value. for the first command in a series of
         ;; piped commands, preivous-value and previous-data will be empty
         {previous-value :value
@@ -56,7 +57,7 @@
                :data-collection previous-data-collection
                :settings (:settings acc)
                :skip-next-n (:skip-next-n acc)
-               :next-cmds next-cmds
+               :next-cmds (and next-cmds-ast (unparse-transformer next-cmds-ast))
                :user *current-user*
                :yetibot-user *yetibot-user*
                :chat-source *chat-source*}

src/yetibot/core/parser.clj

@@ -66,23 +66,3 @@
 
 (defn parse-and-eval [input]
   (-> input parser transformer))
-
-(defn reconstruct-pipe [cmds] (join " | " cmds))
-
-(def unparse-transformer
-  "Takes a parse tree and turns it back into a string"
-  (partial
-    insta/transform
-    {:words str
-     :literal str
-     :space str
-     :parened str
-     :cmd identity
-     ;; avoid wrapping the top level :expr in a subexpression when unparsing
-     :sub-expr str
-     :expr (fn [& cmds] (str "$(" (reconstruct-pipe cmds)  ")"))
-     }))
-
-(defn unparse [parse-tree]
-  (let [cmds (rest parse-tree) ]
-    (reconstruct-pipe (unparse-transformer cmds))))

src/yetibot/core/unparser.clj

@@ -0,0 +1,28 @@
+(ns yetibot.core.unparser
+  (:require
+   [clojure.string :refer [join]]
+   [instaparse.core :as insta]))
+
+(defn reconstruct-pipe [cmds] (join " | " cmds))
+
+(def unparse-transformer
+  "Takes a parse tree and turns it back into a string"
+  (partial
+    insta/transform
+    {:words str
+     :literal str
+     :space str
+     :parened str
+     :cmd identity
+     ;; avoid wrapping the top level :expr in a subexpression when unparsing
+     :sub-expr str
+     :expr (fn [& cmds] (str "$(" (reconstruct-pipe cmds)  ")"))
+     }))
+
+(comment
+  (unparse-transformer
+   [[:cmd [:words "grep" [:space " "] "minutes"]]]))
+
+(defn unparse [parse-tree]
+  (let [cmds (rest parse-tree) ]
+    (reconstruct-pipe (unparse-transformer cmds))))

test/yetibot/core/test/parser.clj

@@ -1,7 +1,7 @@
 (ns yetibot.core.test.parser
   (:require [midje.sweet :refer [=> fact facts]]
-            [yetibot.core.parser :refer [parser unparse
-                                         parse-and-eval]]))
+            [yetibot.core.unparser :refer [unparse]
+            [yetibot.core.parser :refer [parser parse-and-eval]]))
 
 (facts "single commands should be parsed"
   (parser "uptime") => [:expr [:cmd [:words "uptime"]]]