[zile-devel] [ zile-Patches-1821380 ] Correct 2 bugs in indentation

[SourceForge.net]

Patches item #1821380, was opened at 2007-10-27 23:11
Message generated for change (Comment added) made by rrt
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=317089&aid=1821380&group_id=17089

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: Yes
Submitted By: Nobody/Anonymous (nobody)
Assigned to: Nobody/Anonymous (nobody)
Summary: Correct 2 bugs in indentation

Initial Comment:
I have send a mail to the mailing about this problem. As I had no feedback, I 
will send the patch as is. The patch corrects the two following bugs :

- The option 'tab-always-indent' does not seem to have the behaviour explained 
in the documentation. After testing a little, I have seen not difference (I may 
have missed something).

- If you are inserting spaces as tabs, you cannot undo a tab insertion in one 
operation (which is the case in emacs).

Cheers,

Julien (address@hidden)

----------------------------------------------------------------------

>Comment By: Reuben Thomas (rrt)
Date: 2007-10-28 22:51

Message:
Logged In: YES 
user_id=50145
Originator: NO

I've adapted it slightly. Please have a look and make sure you agree!

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2007-10-28 22:40

Message:
Logged In: NO 

> Thanks, I've applied your patch. Please confirm it's OK now, and I can
> release 2.2.49. It's good to have this explanation!

Everything is alright. 
I have pasted a comment patch as I forgot to add these previously and I
had a hard time remembering why I was looking at the previous line. You can
modify it according to your need but I think it is important to add these
explanations.
Thanks for your feedback.

Index: line.c
===================================================================
RCS file: /cvsroot/zile/zile/src/line.c,v
retrieving revision 1.75
diff -u -r1.75 line.c
--- line.c  28 Oct 2007 21:47:17 -0000  1.75
+++ line.c  28 Oct 2007 22:39:15 -0000
@@ -648,6 +648,9 @@
   return ok;
 }

+/*
+ * Return the indentation of the previous line
+ */
 static size_t current_indent(void)
 {
   size_t cur_indent;
@@ -673,6 +676,11 @@
 Indent line or insert a tab.
 +*/
 {
+  /*
+   * If 'tab-always-indent' is nil, compare the column number with the
current indentation
+   * if the result is strictly smaller then indent relatively to the
previous line
+   * if the result is greater then we should indent absolutely
+   */
   if (!lookup_bool_variable("tab-always-indent") && get_goalc() >=
current_indent())
     return insert_tab();
   else



----------------------------------------------------------------------

Comment By: Reuben Thomas (rrt)
Date: 2007-10-28 22:03

Message:
Logged In: YES 
user_id=50145
Originator: NO

Thanks, I've applied your patch. Please confirm it's OK now, and I can
release 2.2.49. It's good to have this explanation!

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2007-10-28 21:23

Message:
Logged In: NO 

> Please can you explain why Zile should look at the previous and not the
> current line?

Here is what I think (correct me if something is wrong in what I write) :

When 'tab-always-indent' is nil, to see how we should indent (relatively
or not), we should compare our column number (which is given by get_goalc)
with the previous line's indentation (that should be given by
current_indent). If the column number is small strictly smaller than the
previous line indentation, we should indent relatively to the previous
line. Otherwise we should indent in an absolute manner.
That is why current_indent should look at the previous line's
indentation.

I should also add that current_indent is only called from
indent-for-tab-command so modifying its behaviour has a very little scope.
I have also done some testing and I could not find a regression with this
modification but I may have missed some test cases.

----------------------------------------------------------------------

Comment By: Reuben Thomas (rrt)
Date: 2007-10-28 18:40

Message:
Logged In: YES 
user_id=50145
Originator: NO

Please can you explain why Zile should look at the previous and not the
current line?

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2007-10-28 01:48

Message:
Logged In: NO 

There was a part I was not really sure about so I removed it but after
testing the CVS version, it is needed. I have pasted the patch with this
reply.


Index: src/line.c
===================================================================
RCS file: /cvsroot/zile/zile/src/line.c,v
retrieving revision 1.74
diff -u -r1.74 line.c
--- src/line.c  27 Oct 2007 23:45:29 -0000      1.74
+++ src/line.c  28 Oct 2007 00:45:00 -0000
@@ -654,7 +654,9 @@
   Marker *save_point = point_marker();

   /* Find first non-blank char. */
+  FUNCALL(previous_line);
   FUNCALL(beginning_of_line);
+
   while (!eolp() && (isspace(following_char())))
     forward_char();



----------------------------------------------------------------------

Comment By: Reuben Thomas (rrt)
Date: 2007-10-28 00:53

Message:
Logged In: YES 
user_id=50145
Originator: NO

This patch looks good, thanks. I've applied it to CVS; could you possibly
test and confirm it works as you expect?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=317089&aid=1821380&group_id=17089