update search-forward / backward to actually use pred

[saikyun]

(varfn search-backward
  "Gets the position of the start of the word before start.
Doesn't skip delimiters in the beginning."
  [gb pred start]
  (var target-i -1)

  (def f (fiber/new (fn [] (gb/index-char-backward-start gb (max 0 (dec start))))))

  (loop [[i c] :iterate (resume f)]
    (when (pred c)
      (break))

    (set target-i i))

  (cond (= -1 target-i)
    start

    (> target-i (gb/gb-length gb))
    (gb/gb-length gb)

    target-i))

(varfn search-forward
  "Gets the position of the end of the word after the start.
Doesn't skip delimiters in the beginning."
  [gb pred start]
  (var target-i -1)

  (def f (fiber/new (fn [] (gb/index-char-start gb start))))

  (loop [[i c] :iterate (resume f)]
    (when (pred c)
      (break))

    (set target-i i))

  (cond (= -1 target-i)
    start

    (> target-i (gb/gb-length gb))
    (gb/gb-length gb)

    # the i we got was the position of the char  
    # the caret position should be one higher than that    
    # i.e. to the right of the char    
    (inc target-i)))

[sogaiu]

The versions in the repository already seem to have:

  (loop [[i c] :iterate (resume f)]
    (when (word-delimiter? c)
      (break))

Do you have some thoughts on how / whether to merge that with what you have above?

  (loop [[i c] :iterate (resume f)]
    (when (pred c)
      (break))

[sogaiu]

Once the situation with the functions above are decided on, may be one of us can add:

(defn index-start-of-line
  ``
  Returns the index of the newline before i.
  ``
  [gb i]
  (search-backward gb |(= $ (chr "\n")) i))

(defn index-end-of-line
  ``
  Returns the index of the newline after i.
  ``
  [gb i]
  (search-forward gb |(= $ (chr "\n")) i))

(varfn beginning-of-line?
  [gb]
  (= (gb :caret) 
     (index-start-of-line gb (gb :caret))))

(varfn end-of-line?
  [gb]
  (= (gb :caret) 
     (index-end-of-line gb (gb :caret))))

(varfn backward-line
  [gb]
  (let [curr-line (index-start-of-line gb (gb :caret))
        # checking not at beginning of buffer not needed
        prev-line (index-start-of-line gb (dec curr-line))]
    (put gb :caret prev-line)))

(varfn forward-line
  [gb]
  (let [curr-line (index-end-of-line gb (gb :caret))
        # checking not at end of buffer not needed
        next-line (inc curr-line)]
    (put gb :caret next-line)))

(varfn beginning-of-line
  [gb]
  (put gb :caret (index-start-of-line gb (gb :caret))))

(varfn end-of-line
  [gb]
  (put gb :caret (index-end-of-line gb (gb :caret))))

[saikyun]

Yeah, the idea is that I need to fix the functions that use search-forward / backward, and have them provide the word-delimiter? as pred.

[sogaiu]

Thanks for the explanation.

I checked briefly but didn't find any place that calls search-forward or search-backward that wasn't already passing word-delimiter? for the value of pred.

There were two comment blocks that had calls: https://github.com/saikyun/freja/blob/c83aafadc326d656055a983420e13ad1cb036172/freja/new_gap_buffer.janet#L340-L350 https://github.com/saikyun/freja/blob/c83aafadc326d656055a983420e13ad1cb036172/freja/new_gap_buffer.janet#L376-L386 and there was this part: https://github.com/saikyun/freja/blob/c83aafadc326d656055a983420e13ad1cb036172/freja/new_gap_buffer.janet#L391-L392

However, it looks like they all pass word-delimiter? already.

[saikyun]

Aha, then it should be fine to make the changes / additions you posted. :) Feel free to make a PR.

Den lör 6 nov. 2021 14:16sogaiu @.***> skrev:

Thanks for the explanation.

I checked briefly but didn't find any place that calls search-forward or search-backward that wasn't already passing word-delimiter? for the value of pred.

There were two comment blocks that had calls: https://github.com/saikyun/freja/blob/c83aafadc326d656055a983420e13ad1cb036172/freja/new_gap_buffer.janet#L340-L350 https://github.com/saikyun/freja/blob/c83aafadc326d656055a983420e13ad1cb036172/freja/new_gap_buffer.janet#L376-L386 and there was this part: https://github.com/saikyun/freja/blob/c83aafadc326d656055a983420e13ad1cb036172/freja/new_gap_buffer.janet#L391-L392

However, it looks like they all pass word-delimiter? already.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/saikyun/freja/issues/54#issuecomment-962450424, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS46Z42EZH2QQJOMHMAZ3TUKUTDTANCNFSM5HN5SJCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[sogaiu]

IIUC, search-backward and search-forward still need to use the predicate though as described in: https://github.com/saikyun/freja/issues/54#issuecomment-962437177

May be that change should happen first?

[saikyun]

Ah, I was thinking they could happen at the same time. But if you want to I can fix that first :)

Den lör 6 nov. 2021 14:59sogaiu @.***> skrev:

IIUC, search-backward and search-forward still need to use the predicate though as described in: #54 (comment) https://github.com/saikyun/freja/issues/54#issuecomment-962437177

May be that change should happen first?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/saikyun/freja/issues/54#issuecomment-962455818, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS46Z5SRW3GPWSHY7X33ALUKUYC5ANCNFSM5HN5SJCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[sogaiu]

If your original change idea is committed separately and first, if for some reason the things I'm thinking of don't work out, they can be backed out of, but your original change idea can still remain more easily.

What do you think?

[saikyun]

I added most functions here: https://github.com/saikyun/freja/commit/0d8310028212e4c3689f75148529a526885752dc

I realized that forward/backward-line could be confusing, since at least I would imagine that the column would stay the same. So for now at least I don't want to put them in new_gap_buffer.

[sogaiu]

Thanks for the changes.

I suppose the forward-line and backward-line names could be confusing. I don't have any better alternatives at the moment.

I think you have something like move-up! and move-down! somewhere -- at least when compared with those (i.e. using up and down), I don't find the names too confusing, though if I didn't know about the others I can imagine the names might not be clear enough.

I wonder if it's worth looking over some of the other names to see how they appear from the perspective of confusion / consistency. Possibly it's better to make changes sooner rather than later :)

[saikyun]

Those functions do keep the column though, so at least they don't do two things. If I were to name the forward-line / backward-line as they are now, I'd call them something like beginning-of-next-line / beginning-of-previous-line.

I agree that naming consistency need to be looked over.

[sogaiu]

Yeah, may be those names you mentioned are better.

One thing I've been wondering is whether using start might be better than beginning. It's shorter for one :)