@techee requested changes on this pull request.


> @@ -219,3 +220,12 @@ void goto_nonempty(ScintillaObject *sci, gint line, 
> gboolean scroll)
                pos = NEXT(sci, pos);
        SET_POS(sci, pos, scroll);
 }
+
+
+gboolean is_start_of_line(ScintillaObject *sci, gint pos)

This function can go directly above the function where it's used and be static 
- there doesn't seem to be any need for it in the rest of the code.

> @@ -219,3 +220,12 @@ void goto_nonempty(ScintillaObject *sci, gint line, 
> gboolean scroll)
                pos = NEXT(sci, pos);
        SET_POS(sci, pos, scroll);
 }
+
+
+gboolean is_start_of_line(ScintillaObject *sci, gint pos)
+{
+       gint line = SSM(sci, SCI_LINEFROMPOSITION, pos, 0);
+       gint line_pos = SSM(sci, SCI_POSITIONFROMLINE, line, 0);
+
+       return pos == line_pos ? TRUE : FALSE;

Drop unnecessary `? TRUE : FALSE`.

> @@ -162,14 +163,14 @@ void cmd_del_word_left(CmdContext *c, CmdParams *p)
 void cmd_undo(CmdContext *c, CmdParams *p)
 {
        gint i;
-       gint pos = SSM(p->sci, SCI_GETCURRENTPOS, 0, 0);
+       start_undo(c);
        for (i = 0; i < p->num; i++)
        {
                if (!SSM(p->sci, SCI_CANUNDO, 0, 0))
                        break;
                SSM(p->sci, SCI_UNDO, 0, 0);
        }

Well, what I had in mind was something like (beware, totally untested!!!):
```C
void cmd_undo(CmdContext *c, CmdParams *p)
{
        gboolean undo_performed = FALSE;
        gint pos;
        gint i;

        undo_start(c);

        for (i = 0; i < p->num; i++)
        {
                if (!SSM(p->sci, SCI_CANUNDO, 0, 0))
                        break;
                SSM(p->sci, SCI_UNDO, 0, 0);
                undo_performed = TRUE;  // <--- this is added
        }

        undo_end(c);

        // the code below is added
        pos = SSM(ctx->sci, SCI_GETCURRENTPOS, 0, 0);

        if (undo_performed && is_start_of_line(p->sci, pos))
                goto_nonempty(p->sci, pos, FALSE);
}
```

without the need to pass anything extra in the context. Am I missing something? 
Since I didn't run or even compile it, it's quite possible.

`excmd_undo()` could just call this function then.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1328#pullrequestreview-2064774399
You are receiving this because you are subscribed to this thread.

Message ID: <geany/geany-plugins/pull/1328/review/2064774...@github.com>

Reply via email to