Fork me on GitHub
#rewrite-clj
<
2019-04-26
>
sogaiu00:04:12

@lee to explore this matter of behavior for out-of-bounds input, i've tried calling find-last-by-pos with some other values. here are some results (i'll repeat some of the set up as the previous values are not logged, but thanks to borkdude, the following should get logged). given:

(require '[rewrite-clj.zip :as rz])
(def a-form "(defn hi-fn\n  [x]\n  (+ x 1))")
it appears that:
(-> a-form (rz/of-string {:track-position? true}) (rz/find-last-by-pos [1 0]) rz/string)
returns nil, but:
(-> a-form (rz/of-string {:track-position? true}) (rz/find-last-by-pos [2 0]) rz/string)
(-> a-form (rz/of-string {:track-position? true}) (rz/find-last-by-pos [3 0]) rz/string) 
return "\n". possibly worth noting, along what you mentioned about returned values passed the end of the form:
(-> a-form (rz/of-string {:track-position? true}) (rz/find-last-by-pos [0 0]) rz/string)
(-> a-form (rz/of-string {:track-position? true}) (rz/find-last-by-pos [4 0]) rz/string)
return nil. it also turns out that using -1 for the column value, yields the same results. i find these return values to be surprising. if we think these are all the same type of situation, may be a single consistent return value makes more sense (e.g. nil) or possibly throwing an exception. not sure though. btw, is the row, col info being 1-based instead of 0-based, documented any where? if it isn't, it seems like it would be worthwhile. wdyt?

lread01:04:15

Thanks @sogaiu, I strongly agree that 1 based needs to be documented very clearly. When I was reviewing your examples, I did not remember it was 1 based and was scratching my head a bit. I’ll have a look at your other scenarios sometime soon, thanks very much for digging in.

lread02:04:37

Wait... [2 0] and [3 0] returning “\n” is not good. I thought that maybe there might be value in distinguishing on or past end of line with “\n” vs past end of form with nil. Given your examples, and the confusion they cause, I think we should throw when <= 0.

lread15:04:09

Ok thanks again @sogaiu, I have updated my spike branch to now throw when any pos is <= 0. I appreciate any further feedback/awkwardnesses you find with this function while you exercise it.