Consider allowing `find-last-by-pos` on non-position tracking zipper

[lread]

Problem/Opportunity

Currently rewrite-clj.zip/find-last-by-pos requires a position tracking zipper. The position tracking zipper tracks and updates positions as the zipper is changed.

A use case came on Slack today where someone wanted to use find-last-by-pos but with a list of locations found by an external tool. The flow is:

• let ex-poses = list of row/cols found by an external tool
• open a zipper in rewrite-clj
• for each pos in ex-poses
  • find node by pos in zipper
  • make a change to zipper

This won't work on a position-tracking zipper. When the zipper is changed, it will update the row/col positions, and therefore, only the first ex-poses will be found.

And it won't work on a non-position because rewrite-clj throws if you attempt to use a find-last-by-pos on a non-position tracking zipper.

Proposed Solution

Allow find-last-by-pos to work on a non-position tracking zipper to support the use above use case.

Alternative Solutions

1. A position tracking zipper could also save original loaded row/rols and we could allow searching by either real row/col or original row/col. But that would be a bigger change API-wise, I think. And also doesn't seem like a better alternative.

Additional context

1. Other zipper functions currently require a position-tracking zipper. They'll need to be reviewed in the same context as find-last-by-pos and updated accordingly.
2. These other functions might be used internally by the paredit API. The paredit API still needs a position-tracking zipper, but its enforcement was likely incidental. We might need to make it explicit.
3. We'll need to make it clear to users what they are getting into if they use pos fns on a non-position-tracking zipper vs a position-tracking zipper. Docstrings will need to be updated. Explicit examples in the user guide will also be helpful.

Action

I can follow up by looking into this and, eventually, write up a PR.

[borkdude]

I vote for the proposed solution. rewrite-clj seems overly protective in this area and prevents otherwise valid use cases. I understand why it was overprotective, but in hindsight it seems like this check could be removed to open up the unforeseen use cases.

[lread]

I still prefer our proposed solution but for completeness, another alternative is to do nothing but document how to satisfy this use case using node metadata on a non-position-tracking-zipper. Example I shared on Slack:

(require '[rewrite-clj.node :as n]
         '[rewrite-clj.zip :as z])

(def s (str "(some code here
that I want
to change)"))

;; some locations found by an external tool
(def locs [[1 7] ;; code
           [2 1] ;; that
           [2 6] ;; I
           [3 4] ;; change
           ])

;; one way to write a find-by-pos for a non-tracking zipper
(defn find-by-pos [zloc [frow fcol]]
  (z/find zloc z/next (fn [zloc]
                        (let [{:keys [row col]} (meta (z/node zloc))]
                          (and (= frow row)
                               (= fcol col))))))

(def zloc (z/of-string s))

;; for demo purposes make each item at loc a fn call with loc as as args
(z/print-root (reduce (fn [zloc loc]
                        (let [zloc (find-by-pos zloc loc)]
                          (z/replace zloc (n/list-node [(z/node zloc)
                                                        (n/spaces 1)
                                                        (first loc)
                                                        (n/spaces 1)
                                                        (second loc)]))))
                      zloc
                      locs))

Outputs:

(some (code 1 7) here
(that 2 1) (I 2 6) want
to (change 3 4))