b4n requested changes on this pull request.
Looks mostly good, but I'd rather not rely on where the caret is. Not relying
on it makes it fairly easy to add a right-click version of the feature (I have
an unpolished version locally that work just fine, yay :)).
Otherwise, it seems to work great :+1:
> +
+ if (data->found) {
+ ScintillaObject *sci = doc->editor->sci;
+
+ sci_start_undo_action(sci);
+
+ if (data->new_lines > 0) {
+ gint pos = sci_get_position_from_line(sci, data->new_start - 1);
+ sci_set_target_start(sci, pos);
+ pos = sci_get_position_from_line(sci, data->new_start +
data->new_lines - 1);
+ sci_set_target_end(sci, pos);
+ sci_replace_target(sci, "", FALSE);
+ }
+
+ if (data->old_lines > 0) {
+ gint line = sci_get_current_line (sci);
Why current line? couldn't it be `gint pos = sci_get_position_from_line (sci,
data->new_start - 1);` instead? I don't get exactly how you can be sure it's
the current line, and not relying on that would allow for that to be used on
non-current lines too (like adding an entry to the editor menu).
And I don't get why caring about position of the delete marker as we don't use
them here, do we?
> +
+ if (data->old_lines > 0) {
+ gint line = sci_get_current_line (sci);
+ gint pos;
+
+ if (data->new_lines == 0 && !data->first_line_removed) {
+ line++; /* marker for deleted hunk is on previous line except the
1st line */
+ }
+ pos = sci_get_position_from_line (sci, line);
+
+ insert_buf_range (doc, contents, pos,
+ data->old_start - 1,
+ data->old_lines);
+ }
+
+ sci_scroll_caret(sci);
Similarly, what about
```C
scintilla_send_message (sci, SCI_SCROLLRANGE,
sci_get_position_from_line (sci, data->new_start
+ data->old_lines - 1),
sci_get_position_from_line (sci, data->new_start
- 1));
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/531#pullrequestreview-37969835
