Skip to content
This repository has been archived by the owner on Jul 7, 2024. It is now read-only.

Commit

Permalink
View: fix bug [Alt+Down or Alt+Up] followed by Ctrl+z
Browse files Browse the repository at this point in the history 
The bug was that the undo operation (Ctrl+z) moved the cursor to the
last line of the buffer.

When moving line(s) down or up (with Alt+Down or Alt+Up), there is the
special case of the buffer end that doesn't end with a newline
character.

The previous solution basically canonicalized the buffer by always
having a trailing newline. Then removing the trailing newline if it was
inserted.

So now, to fix the bug, a trailing newline is inserted *only* when the
end of the buffer is involved.

Basic regression/unit tests are included, so this bug should hopefully
not re-appear.

Fixes: https://gitlab.gnome.org/GNOME/gedit/-/issues/343
  • Loading branch information
@swilmet
swilmet committed Dec 5, 2023
1 parent 7062264 commit 8162e82
Show file tree
Hide file tree
Showing 2 changed files with 240 additions and 63 deletions.
218 changes: 155 additions & 63 deletions gtksourceview/gtksourceview.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3697,19 +3697,6 @@ gtk_source_view_move_words (GtkSourceView *view,
g_free (new_text);
}

static gboolean
buffer_contains_trailing_newline (GtkTextBuffer *buffer)
{
GtkTextIter iter;
gunichar ch;

gtk_text_buffer_get_end_iter (buffer, &iter);
gtk_text_iter_backward_char (&iter);
ch = gtk_text_iter_get_char (&iter);

return (ch == '\n' || ch == '\r');
}

/* FIXME could be a function of GtkSourceBuffer, it's also useful for the
* FileLoader.
*/
Expand Down Expand Up @@ -3737,101 +3724,206 @@ remove_trailing_newline (GtkTextBuffer *buffer)
}

static void
gtk_source_view_move_lines (GtkSourceView *view,
gboolean down)
move_lines_up (GtkTextBuffer *buffer)
{
GtkTextBuffer *buffer;
GtkTextIter start;
GtkTextIter end;
GtkTextIter insert_pos;
GtkTextIter selection_iter_start;
GtkTextIter selection_iter_end;
GtkTextMark *start_mark;
GtkTextMark *end_mark;
gboolean trailing_newline_inserted = FALSE;
gchar *text;
gboolean initially_contains_trailing_newline;
GtkTextIter insert_pos;

buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
/* start and end are set in ascending order. */
gtk_text_buffer_get_selection_bounds (buffer, &selection_iter_start, &selection_iter_end);

if (!gtk_text_view_get_editable (GTK_TEXT_VIEW (view)))
/* Move to start of line for the beginning of the selection.
* Entire lines must be moved.
*/
gtk_text_iter_set_line_offset (&selection_iter_start, 0);

if (gtk_text_iter_is_start (&selection_iter_start))
{
/* Nothing to do, and the undo/redo history must remain unchanged. */
return;
}

gtk_text_buffer_get_selection_bounds (buffer, &start, &end);

/* Get the entire lines, including the paragraph terminator. */
gtk_text_iter_set_line_offset (&start, 0);
if (!gtk_text_iter_starts_line (&end) ||
gtk_text_iter_get_line (&start) == gtk_text_iter_get_line (&end))
if (!gtk_text_iter_starts_line (&selection_iter_end) ||
gtk_text_iter_get_line (&selection_iter_start) == gtk_text_iter_get_line (&selection_iter_end))
{
gtk_text_iter_forward_line (&end);
gtk_text_iter_forward_line (&selection_iter_end);
}

if ((!down && gtk_text_iter_is_start (&start)) ||
(down && gtk_text_iter_is_end (&end)))
gtk_text_buffer_begin_user_action (buffer);

/* We must be careful about what operations we do on the GtkTextBuffer,
* for the undo/redo.
*/

/* Insert a trailing newline, but only if necessary. */
if (gtk_text_iter_is_end (&selection_iter_end) &&
(gtk_text_iter_get_line (&selection_iter_start) == gtk_text_iter_get_line (&selection_iter_end) ||
!gtk_text_iter_starts_line (&selection_iter_end)))
{
/* Nothing to do, and the undo/redo history must remain
* unchanged.
*/
return;
start_mark = gtk_text_buffer_create_mark (buffer, NULL, &selection_iter_start, TRUE);

gtk_text_buffer_insert (buffer, &selection_iter_end, "\n", -1);
trailing_newline_inserted = TRUE;

gtk_text_buffer_get_iter_at_mark (buffer, &selection_iter_start, start_mark);
gtk_text_buffer_delete_mark (buffer, start_mark);
start_mark = NULL;
}

start_mark = gtk_text_buffer_create_mark (buffer, NULL, &start, TRUE);
end_mark = gtk_text_buffer_create_mark (buffer, NULL, &end, FALSE);
text = gtk_text_buffer_get_text (buffer, &selection_iter_start, &selection_iter_end, TRUE);

gtk_text_buffer_begin_user_action (buffer);
gtk_text_buffer_delete (buffer, &selection_iter_start, &selection_iter_end);

initially_contains_trailing_newline = buffer_contains_trailing_newline (buffer);
insert_pos = selection_iter_start;
gtk_text_iter_backward_line (&insert_pos);

if (!initially_contains_trailing_newline)
{
/* Insert a trailing newline. */
gtk_text_buffer_get_end_iter (buffer, &end);
gtk_text_buffer_insert (buffer, &end, "\n", -1);
}
start_mark = gtk_text_buffer_create_mark (buffer, NULL, &insert_pos, TRUE);

/* At this point all lines finish with a newline or carriage return, so
* there are no special cases for the last line.
*/
gtk_text_buffer_insert (buffer, &insert_pos, text, -1);
g_free (text);

gtk_text_buffer_get_iter_at_mark (buffer, &start, start_mark);
gtk_text_buffer_get_iter_at_mark (buffer, &end, end_mark);
/* Select the moved text. */
gtk_text_buffer_get_iter_at_mark (buffer, &selection_iter_start, start_mark);
gtk_text_buffer_delete_mark (buffer, start_mark);
gtk_text_buffer_delete_mark (buffer, end_mark);
start_mark = NULL;
end_mark = NULL;

text = gtk_text_buffer_get_text (buffer, &start, &end, TRUE);
gtk_text_buffer_select_range (buffer, &selection_iter_start, &insert_pos);

if (trailing_newline_inserted)
{
remove_trailing_newline (buffer);
}

gtk_text_buffer_end_user_action (buffer);
}

gtk_text_buffer_delete (buffer, &start, &end);
static gboolean
can_move_lines_down (GtkTextBuffer *buffer,
const GtkTextIter *selection_iter_start,
const GtkTextIter *selection_iter_end)
{
GtkTextIter end_iter;

if (down)
gtk_text_buffer_get_end_iter (buffer, &end_iter);

if (gtk_text_iter_get_line (selection_iter_end) != gtk_text_iter_get_line (&end_iter))
{
insert_pos = end;
gtk_text_iter_forward_line (&insert_pos);
return TRUE;
}
else

/* Now we know that 'selection_iter_end' is on the last line. */

return (gtk_text_iter_get_line (selection_iter_start) != gtk_text_iter_get_line (selection_iter_end) &&
gtk_text_iter_starts_line (selection_iter_end));
}

static void
move_lines_down (GtkTextBuffer *buffer)
{
GtkTextIter selection_iter_start;
GtkTextIter selection_iter_end;
gchar *text;
GtkTextIter insert_pos;
GtkTextIter end_iter;
GtkTextMark *start_mark;
gboolean trailing_newline_inserted = FALSE;

/* start and end are set in ascending order. */
gtk_text_buffer_get_selection_bounds (buffer, &selection_iter_start, &selection_iter_end);

if (!can_move_lines_down (buffer, &selection_iter_start, &selection_iter_end))
{
/* Nothing to do, and the undo/redo history must remain unchanged. */
return;
}

/* Move to start of line for the beginning of the selection.
* Entire lines must be moved.
*/
gtk_text_iter_set_line_offset (&selection_iter_start, 0);

/* Get the entire lines, including the paragraph terminator. */
if (!gtk_text_iter_starts_line (&selection_iter_end) ||
gtk_text_iter_get_line (&selection_iter_start) == gtk_text_iter_get_line (&selection_iter_end))
{
insert_pos = start;
gtk_text_iter_backward_line (&insert_pos);
gtk_text_iter_forward_line (&selection_iter_end);
}

gtk_text_buffer_begin_user_action (buffer);

/* We must be careful about what operations we do on the GtkTextBuffer,
* for the undo/redo.
*/

text = gtk_text_buffer_get_text (buffer, &selection_iter_start, &selection_iter_end, TRUE);

gtk_text_buffer_delete (buffer, &selection_iter_start, &selection_iter_end);

insert_pos = selection_iter_end;

/* Insert a trailing newline, but only if necessary. */
gtk_text_buffer_get_end_iter (buffer, &end_iter);
if (gtk_text_iter_get_line (&insert_pos) == gtk_text_iter_get_line (&end_iter))
{
start_mark = gtk_text_buffer_create_mark (buffer, NULL, &insert_pos, TRUE);

gtk_text_buffer_insert (buffer, &end_iter, "\n", -1);
trailing_newline_inserted = TRUE;

gtk_text_buffer_get_iter_at_mark (buffer, &insert_pos, start_mark);
gtk_text_buffer_delete_mark (buffer, start_mark);
start_mark = NULL;
}

gtk_text_iter_forward_line (&insert_pos);

start_mark = gtk_text_buffer_create_mark (buffer, NULL, &insert_pos, TRUE);

gtk_text_buffer_insert (buffer, &insert_pos, text, -1);
g_free (text);

/* Select the moved text. */
gtk_text_buffer_get_iter_at_mark (buffer, &start, start_mark);
gtk_text_buffer_get_iter_at_mark (buffer, &selection_iter_start, start_mark);
gtk_text_buffer_delete_mark (buffer, start_mark);
start_mark = NULL;

gtk_text_buffer_select_range (buffer, &start, &insert_pos);
gtk_text_buffer_select_range (buffer, &selection_iter_start, &insert_pos);

if (!initially_contains_trailing_newline)
if (trailing_newline_inserted)
{
remove_trailing_newline (buffer);
}

gtk_text_buffer_end_user_action (buffer);
}

static void
gtk_source_view_move_lines (GtkSourceView *view,
gboolean down)
{
GtkTextBuffer *buffer;

if (!gtk_text_view_get_editable (GTK_TEXT_VIEW (view)))
{
return;
}

buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));

/* Split the two cases, otherwise the code is messier. */
if (down)
{
move_lines_down (buffer);
}
else
{
move_lines_up (buffer);
}

gtk_text_view_scroll_mark_onscreen (GTK_TEXT_VIEW (view),
gtk_text_buffer_get_insert (buffer));
Expand Down
85 changes: 85 additions & 0 deletions tests/unit-tests/test-view.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,89 @@ test_move_lines__move_several_lines (void)
g_object_unref (view);
}

/* There was a bug with the undo operation that moved the cursor to the last
* line of the buffer, even if the moved line(s) were unrelated to the end of
* the buffer. That was problematic for lengthy files, of course.
*/
static void
test_move_line_down_then_undo (void)
{
GtkSourceView *view;
GtkTextBuffer *buffer;
GtkTextIter start_iter;
GtkTextIter end_iter;
GtkTextIter selection_start_iter;
GtkTextIter selection_end_iter;

view = GTK_SOURCE_VIEW (gtk_source_view_new ());
g_object_ref_sink (view);

buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
gtk_text_buffer_set_text (buffer,
"line1\n"
"line2\n"
"line3\n"
"line4",
-1);

/* Move the first line down. */
gtk_text_buffer_get_start_iter (buffer, &start_iter);
gtk_text_buffer_place_cursor (buffer, &start_iter);
g_signal_emit_by_name (view, "move-lines", TRUE);

/* Undo. */
g_assert_true (gtk_source_buffer_can_undo (GTK_SOURCE_BUFFER (buffer)));
gtk_source_buffer_undo (GTK_SOURCE_BUFFER (buffer));

/* The cursor must not have been moved to the last line. */
gtk_text_buffer_get_selection_bounds (buffer, &selection_start_iter, &selection_end_iter);
gtk_text_buffer_get_end_iter (buffer, &end_iter);
g_assert_cmpint (gtk_text_iter_get_line (&selection_start_iter), !=, gtk_text_iter_get_line (&end_iter));
g_assert_cmpint (gtk_text_iter_get_line (&selection_end_iter), !=, gtk_text_iter_get_line (&end_iter));

g_object_unref (view);
}

/* See the comment for test_move_line_down_then_undo(). */
static void
test_move_line_up_then_undo (void)
{
GtkSourceView *view;
GtkTextBuffer *buffer;
GtkTextIter iter;
GtkTextIter end_iter;
GtkTextIter selection_start_iter;
GtkTextIter selection_end_iter;

view = GTK_SOURCE_VIEW (gtk_source_view_new ());
g_object_ref_sink (view);

buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
gtk_text_buffer_set_text (buffer,
"line1\n"
"line2\n"
"line3\n"
"line4",
-1);

/* Move the second line up. */
gtk_text_buffer_get_iter_at_line (buffer, &iter, 1);
gtk_text_buffer_place_cursor (buffer, &iter);
g_signal_emit_by_name (view, "move-lines", FALSE);

/* Undo. */
g_assert_true (gtk_source_buffer_can_undo (GTK_SOURCE_BUFFER (buffer)));
gtk_source_buffer_undo (GTK_SOURCE_BUFFER (buffer));

/* The cursor must not have been moved to the last line. */
gtk_text_buffer_get_selection_bounds (buffer, &selection_start_iter, &selection_end_iter);
gtk_text_buffer_get_end_iter (buffer, &end_iter);
g_assert_cmpint (gtk_text_iter_get_line (&selection_start_iter), !=, gtk_text_iter_get_line (&end_iter));
g_assert_cmpint (gtk_text_iter_get_line (&selection_end_iter), !=, gtk_text_iter_get_line (&end_iter));

g_object_unref (view);
}

int
main (int argc,
char **argv)
Expand All @@ -548,6 +631,8 @@ main (int argc,

g_test_add_func ("/view/move-lines/move-single-line", test_move_lines__move_single_line);
g_test_add_func ("/view/move-lines/move-several-lines", test_move_lines__move_several_lines);
g_test_add_func ("/view/move-lines/move-line-down-then-undo", test_move_line_down_then_undo);
g_test_add_func ("/view/move-lines/move-line-up-then-undo", test_move_line_up_then_undo);

ret = g_test_run ();
gtk_source_finalize ();
Expand Down

0 comments on commit 8162e82

Please sign in to comment.