After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 624963 - (Still) Incorrect line shifting
(Still) Incorrect line shifting
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
3.99.x
Other Linux
: Normal normal
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2010-07-21 19:41 UTC by qforce
Modified: 2017-03-30 15:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This patch fixes the bug, in the gtksourceview for the change of lines Cleaning the \n from buffer (3.06 KB, patch)
2017-03-21 19:25 UTC, anthony
needs-work Details | Review

Description qforce 2010-07-21 19:41:52 UTC
I reported a line shifting bug in an earlier bug report: http://bugzilla.gnome.org/show_bug.cgi?id=601889

After having seen the bugfix in gedit 2.30.3 on Ubuntu 10.04, I believe the current behavior is still not (entirely) correct. For example, if the text file contains the following 3 lines:

first line
second line
third line

and I press Alt + Arrow Up on the third line, the third and the second line are swapped correctly. What's incorrect is that gedit also adds an empty fourth line after the third line.

I find this behavior slightly irritating, but of course that's only a minor issue.
Thanks for listening and for the hard work :)
Comment 1 Ignacio Casal Quinteiro (nacho) 2011-04-22 16:12:44 UTC
I can confirm this. I would have to investigate what's producing this.
Comment 2 Sébastien Wilmet 2015-04-27 16:01:53 UTC
Still a problem with gedit 3.14.
Comment 3 anthony 2017-02-13 17:34:23 UTC
I try but , i  dont know if the code "line shifting" (shortcut : alt +up) is in the source code of gedit (I have not found any line that mentions) or if it is gtk thing, I have done a bit of compilation of the bug

http://akanooblinux.blogspot.pe/
Comment 4 anthony 2017-03-21 19:25:57 UTC
Created attachment 348438 [details] [review]
This patch fixes the bug, in the gtksourceview for the change of lines Cleaning the \n from buffer
Comment 5 Fabio Durán Verdugo 2017-03-22 12:47:22 UTC
I can confirm this patch resolves the bug, I apply the patch to last gtksourceview code and testing the behavior and is better.

I reassign to GtkSourceView because this issues too affects to other GNOME Apps like Builder and Latexila.
Comment 6 Sébastien Wilmet 2017-03-29 13:42:10 UTC
Review of attachment 348438 [details] [review]:

Thanks for the patch. Even if it works, it is not of sufficiently good quality to be included as-is.

First, the patch needs to be generated with the git format-patch command, read the README and HACKING files.

::: gtksourceview/gtksourceview.c
@@ +3692,3 @@
 	gboolean down;
 	gchar *text;
+	gboolean First_case = 0;

variable names should be in lowercase. As a general rule of thumb, use the same coding style as the current file (it is explained in the HACKING file).

Also, a gboolean can receive the FALSE and TRUE values, more meaningful than 0 and 1.

@@ +3789,3 @@
 	{
 		gtk_text_iter_backward_line (&e);
+		if( First_case )

It sucks to have a big block of code for the special case. I think it's possible to implement this function more easily.

@@ -3793,3 @@
 	gtk_text_buffer_delete_mark (buf, mark);
 }
-

Apparently you removed a line that you should not have removed. Please read the diff more carefully when submitting a patch.
Comment 7 Sébastien Wilmet 2017-03-29 17:07:59 UTC
I've started to write unit tests.

I think that for the handling of the final newline, the code can be simplified with something along those lines:

gboolean initially_contains_final_newline = ...;

if (!initially_contains_final_newline)
{
  /* insert a final newline */
}

/* Do the change, at this point the buffer always contains a final newline, so the implementation will be simpler. */

if (!initially_contains_final_newline)
{
  /* remove the final newline */
}
Comment 8 Sébastien Wilmet 2017-03-30 15:33:08 UTC
Fixed:
commit bc554dcac5a4f5b80918b7584c37f8e70342b5b0