namespace caching breaks updating namespace name

[dpsutton]

 (defun clojure-update-ns ()
      "Update the namespace of the current buffer.
    Useful if a file has been renamed."
      (interactive)
      (let ((nsname (funcall clojure-expected-ns-function)))
        (when nsname
          (save-excursion
            (save-match-data
              (if (clojure-find-ns)
                  (progn
                    (replace-match nsname nil nil nil 4)
                    (message "ns form updated to `%s'" nsname)
                    (setq clojure-cached-ns nsname))
                (error "Namespace not found")))))))

This is the helpful function to update the namespace and reset the cache for namespaces. Unfortunately it relies on the replace match generated in clojure-find-ns. The problem is that this match does not happen if namespace caching is enbled.

    (defun clojure-find-ns ()
      "Return the namespace of the current Clojure buffer.
    Return the namespace closest to point and above it.  If there are
    no namespaces above point, return the first one in the buffer.

    The results will be cac(match-string-no-properties 4)hed if `clojure-cache-ns' is set to t."
      (if (and clojure-cache-ns clojure-cached-ns)
          clojure-cached-ns
        (let ((ns (save-excursion
                    (save-restriction
                      (widen)

                      ;; Move to top-level to avoid searching from inside ns
                      (ignore-errors (while t (up-list nil t t)))

                      ;; The closest ns form above point.
                      (when (or (re-search-backward clojure-namespace-name-regex nil t)
                                ;; Or any form at all.
                                (and (goto-char (point-min))
                                     (re-search-forward clojure-namespace-name-regex nil t)))
                        (match-string-no-properties 4))))))
          (setq clojure-cached-ns ns)
          ns)))

Note the top level bails if the cache is already set and therefore does not call (match-string-no-properties 4) yielding an error.

This leaves an unfortunate catch-22: if you use the cache, you need to update it with update-ns. If you update the ns with the cache on, it doesn't create the match to update the cache.

[dpsutton]

This also affects clojure-sort-ns except not tragically. It calls (comment-normalize-vars) at the top which creates match data which usually coincides with the ns form. But if you put another form before the ns form this breaks.

[dpsutton]

This also affects CIDER as well.

    (defun cider-eval-ns-form ()
      "Evaluate the current buffer's namespace form."
      (interactive)
      (when (clojure-find-ns)
        (save-excursion
          (goto-char (match-beginning 0))
          (cider-eval-defun-at-point))))

    (defun cider-ns-form ()
      "Retrieve the ns form."
      (when (clojure-find-ns)
        (save-excursion
          (goto-char (match-beginning 0))
          (cider-defun-at-point))))

The caching breaks the contract of side-effecting the match data unfortunately.

[vspinu]

Extracting the search logic into clojure-goto-ns would be a way to go here.

[bbatsov]

Yeah, probably we need two functions indeed - one that finds the form and another that simply returns the current namespace. Right now things are way too convoluted.

[bbatsov]

@dpsutton Interested in tackling this?

I was thinking of adding a clojure-current-ns function that the caching would only apply to and rename the current function to clojure-find-ns-form (or something like this). There are two approaches we can take with it:

• have it return some richer datastructure like the location of the ns form and it's various components. We should also add another function that can modify an ns form based on this data.
• keep the current behaviour
• something better I can't think of :-)

I'd really love for us to get rid of relying on the implementation details and the regexp matching, that's for sure. Won't have time for this for a while, but as it's not complex and you're pretty familiar with the matter I hope you'd "volunteer" to improve this. 😉