Fix browser-cookies error in clientOnly mode

[betoharres]

Fix #77 without tests It fix clientOnly mode parsing undefined variables and fix modals not showing because of undefined variables Check bed6b3b commit message for more info

[tuhdo]

Thanks for the detailed report. Could you give the sample file above?

[tuhdo]

I've fixed the indenting and indent-tab-mode issues. Please check again when it's ready on MELPA (or you can just copy the file from there and eval again).

[tsdh]

Hi @tuhdo,

the file I used for testing is https://github.com/jgralab/funnyqt/blob/master/src/funnyqt/in_place.clj. I use aggressive-indent-mode so all files in that project should be indented exactly as clojure-mode does it (with the exception of custom macros where I have some define-clojure-indent configs in my ~/.emacs).

I've tested the latest version, and the indent-tabs-mode commit indeed already fixes many issues from above.

[tuhdo]

Meanwhile waiting for it to be fixed, you can use srefactor-lisp-format-sexp to format current sexp at point when there is a let in your function body.

[tuhdo]

These are solved so far:

 (ns funnyqt.in-place
-  "In-place transformation stuff."
-  (:require [clojure.tools.macro   :as m]
-            [funnyqt.generic       :as g]
-            [funnyqt.visualization :as viz]
-            [funnyqt.utils         :as u]
-            [funnyqt.query         :as q]
-            [funnyqt.pmatch        :as pm]
-            [funnyqt.tg            :as tg])
-  (:import
-   (javax.swing JDialog JButton AbstractAction WindowConstants BoxLayout
-                JPanel JLabel JScrollPane JComboBox Action JCheckBox
-                Box BorderFactory ImageIcon JComponent)
-   (java.awt.event ActionEvent ItemEvent ItemListener)
-   (java.awt Color GridLayout GridBagLayout GridBagConstraints)))
+    "In-place transformation stuff."
+    (:require [clojure.tools.macro :as m]
+         [funnyqt.generic :as g]
+         [funnyqt.visualization :as viz]
+         [funnyqt.utils :as u]
+         [funnyqt.query :as q]
+         [funnyqt.pmatch :as pm]
+         [funnyqt.tg :as tg])
+    (:import (javax.swing JDialog JButton AbstractAction WindowConstants BoxLayout JPanel JLabel JScrollPane JComboBox Action JCheckBox Box BorderFactory ImageIcon JComponent)
+        (java.awt.event ActionEvent ItemEvent ItemListener)
+        (java.awt Color GridLayout GridBagLayout GridBagConstraints)))

 (def ^{:dynamic true
-       :doc "Only for internal use.  See `as-pattern' macro."}
-  *as-pattern* false)
+     :doc "Only for internal use.  See `as-pattern' macro."
+     } *as-pattern* false)

-(defmacro as-pattern
-  "Performs the given rule application `rule-app` as a pattern.
+(defmacro as-pattern "Performs the given rule application `rule-app` as a pattern.

-(defmacro letrule
-  "Establishes local rules just like `letfn` establishes local fns.
+(defmacro letrule "Establishes local rules just like `letfn` establishes local fns.
   Also see `rule` and `defrule`."
-  {:arglists '([[rspecs] & body])}
+  {:arglists '
+  ([[rspecs]
+    &
+    body])
+  }

   `(binding [*as-pattern* true]
-     ~rule-app))
+       ~rule-app))

It works well enough for Emacs Lisp, Common Lisp and Scheme. Clojure syntax is more complicated though so it's more difficult to handle. But good nevertheless, since if it can handle Clojure code it should handle Lisps fine. The most difficult one is the let, I will leave it last.

[tsdh]

Good progress so far. I guess the main problem with Clojure is that its binding forms (let, binding) pair the variables and expressions without explicitly wrapping them in parenthesis, right?

[tuhdo]

That's right. To solve this problem, I will add another custom variable, each element is a list of a head symbol (ilke let), the position of sexp inside the sexp of the head symbol to be specially formatted, and a formatter function for that particular sexp instead of current generic sexp formatter.

[tuhdo]

@tsdh Now these are solved:

 (defn ^:private unrecur
   "Replaces (recur ...) forms with (fnname ...) forms where *as-test* is bound to false.
   Existing (fnname ...) forms are also wrapped by bindings of *as-test* to
   false.  Doesn't replace in nested `loop` or `fn` forms."
   [fnname form]
-  (u/prewalk (fn [el]
-               (if (and (seq? el)
-                        (or (= (first el) 'recur)
-                            (= (first el) fnname)))
-                 `(binding [*as-test* false]
-                    (~fnname ~@(next el)))
-                 el))
-             (fn [el]
-               (and (seq? el)
-                    (let [x (first el)]
-                      (or (= x `clojure.core/loop)
-                          (= x `clojure.core/fn)))))
-             form))
+  (u/prewalk
+   (fn [el]
+       (if (and (seq? el)
+       (or (= (first el) 'recur)
+           (= (first el) fnname)))
+      `(binding
+        [*as-test* false]
+        (~fnname ~@
+             (next el)))
+    el))
+   (fn [el]
+       (and (seq? el)
+       (let [x (first el)]
+         (or (= x `clojure.core/loop)
+         (= x `clojure.core/fn)))))
+   form))

+           (try (f)
+            (catch Exception
+              e
+              (.printStackTrace e))))))

The above erroneous case of catch above is easy to handle. All you need to do is add it to the skip symbol list like in this commit: https://github.com/tuhdo/semantic-refactor/commit/7e235f748267809abd1f958f943d7eafdeb248df. 2 means that Srefactor skipos two symbols to the right of the head symbols before it inserts a newline. If you have more cases like this, add it and PR is welcome.

I think it's pretty good to use at a small scale like format an sexp tree, i.e. when you delete and add sexp and the layout becomes a bit messy, just run srefactor-lisp-format-sexp.

[tuhdo]

Now only the let and the handling of closing } left.

[tuhdo]

@tsdh what is this called in Clojure?

{1 2, 3 4, 5 6, 7 8}

[tsdh]

Tu Do [email protected] writes:

@tsdh what is this called in Clojure?

{1 2, 3 4, 5 6, 7 8}

It's a map literal, i.e., a hash-map where the keys are 1, 3, 5, and 7 and the associated values are 2, 4, 6, and 8.

[tuhdo]

@tsdh the trailing bracket:

+     :doc "Only for internal use.  See `as-pattern' macro."
+     } *as-pattern* false)

is solved. Now only the let formatting left.

[tsdh]

:+1:

[tuhdo]

The formatting is getting better over the past week. However, to implement this last case, it will take a while because now I don't have much time anymore. In addition, I want to make it right: instead of manually hard coded the form of sexp, I let user define and customize it like this, for example, the let form:

(srefactor-deftemplate :major-mode clojure-mode
                       :form (let (:newline-gap 2 :align-regexp ":")
                               ))

which means that the second sexp of the form let will insert a newline every 2 sexp (either a symbol or a list) and align according to regex ":" if there's one.

[tsdh]

That looks like a good idea. (I don't get why there should be a ":" in a let-binding form, though.)

And just to give you another challenge, this is also a valid let-form:

(let [x 1
      y 2 #_(this is a form not read by the reader)
      z 17]
  ...)

Any form in clojure can be "commented out" by prefixing it with #_. So such forms have to be completely ignored when inserting a newline every two sexps. However, such a #_sexp form may be arbitrarily complex and should be subject to formatting itself. And of course #_ forms may be nested. :-)

[tuhdo]

That looks like a good idea. (I don't get why there should be a ":" in a let-binding form, though.)

So that it looks like keyword arguments and it's easier to quickly separate keywords and value with a glimpse.

And thanks for the input.

[tsdh]

@tuhdo Sorry, I still don't get it. In a clojure let binding-vector there cannot be any colons to which one could align. The value forms can contain colons, of course, but I don't see why those would provide a sensible point for alignment.

Can you simply provide a clojure code snippet showing what you have in mind?

[tuhdo]

@tsdh the template is not the actual code but formatting spec for a specific sexp in the form. Let's look at this again:

(srefactor-deftemplate :major-mode clojure-mode
                       :form (let [:newline-gap 2 :align-regexp ":"]
                               ))

The form [:newline-gap 2 :align-regexp ":"] you are seeing is not actual Clojure form but merely a format specification to tell Srefactor to apply such formatting to the second form after the let keyword. :newline-gap 2 means that for every 2 sexp in that form, a newline is inserted. Let's have a look at another example:

(srefactor-deftemplate :major-mode common-lisp-mode
                       :form (cond
                              ((:newline-gap 1))
                               ))

It means that the conditional form of the cond will always be inserted with newline after the first sexp, no matter what.

However, I've also thought of another style of formatting to handle cases like loop macro in Common Lisp. For example:

(srefactor-deftemplate :major-mode common-lisp-mode
                       :form (loop
                              for :s in :s :nl
                              for :s from :s to :s :nl
                              do :s
))

When Srefactor see :s, replace it with the actual sexp in the token stream. When Srefactor see :nl, it inserts a newline. For every other symbols in the template, just insert it as is. Of course loop macro has many variants so we must have different templates. The let binding can be also rewritten in this format:

(srefactor-deftemplate :major-mode clojure-mode
                       :form (let [:s :s :nl :align-regexp ":"]
                               ))

It has the same meaning as above, and :align-regexp tells Srefactor to align according to ":".

[tsdh]

@tuhdo Yes, I understand that this is a template for srefactor and no clojure form. What I'm asking for is a sample clojure let-form where the :align-regexp ":" would have an effect (and what the effect would be). ;-)

[tuhdo]

Ah I see. Proper example should be:

(srefactor-deftemplate :major-mode clojure-mode
                       :form (:require [:s :s :nl :align-regexp ":"]
                               ))

[tsdh]

@tuhdo Hehe, you still didn't get me. The above template says that the :require form of a clojure ns form should be aligned at :. So I guess I get an indentation like

(ns my.ns
  (:require [foo.bar.baz :as baz]
             [quux.bla   :as bla]))

so the :as keywords are aligned. That's very cool!

BUT: you also specified :align-regexp ":" in the template for the clojure let form. But in a let form there is usually no point in aligning at a :. Usually, there are no colons at all.

If you wanted to align a let binding form, then you'd align it to the start of the value-expression of the variable with the longest name, e.g.,

(let [x      19
      foo    (do-it :x x)
      z      (some-other-expr)
      foobar 1000]
  ...)

However, I don't know if that's a good idea as default behavior. It's only nice if the let isn't indented very deeply, the variable names are reasonable short, and so are the value expressions.

[allentiak]

Hi, @tuhdo ,

I guess the best would be for the behavior to be based on the clojure style guide.

Best, Leandro