bug#10906: 24.0.94; `c-mark-function' does set the mark well

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Dani Moncayo]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

Recipe from "emacs -Q":
1. Visit the attached file.
2. Move point to the end of the "printf" sentence.
3. Type C-M-h
4. C-g
5. Type C-u C-SPC

--> Expected result: The point moves back to the position it occupied
just before step #3, i.e., to the end of the "printf" sentence.  This
is the behavior that makes sense, and the one you get if you replace
steps #3 and #4 with "Type C-M-a".

--> Observed result: The point moves to the end of the "main" function.


In GNU Emacs 24.0.94.1 (i386-mingw-nt6.1.7601)
 of 2012-02-27 on DANI-PC
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --with-gcc (4.6)'


-- 
Dani Moncayo

[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 96 bytes --]


// Test
void main(void)
{
  // Say hello
  printf("Hello, world!");
}

// End of file

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Dani Moncayo]

On Wed, Feb 29, 2012 at 01:14, Juri Linkov <juri@jurta.org> wrote:
>[...]
> Regarding bug#10906, I think `c-mark-function' should be rewritten
> to follow the logic of `mark-defun'.

Agreed, and BTW, if `c-mark-function' is going to be revised, please,
take also this problem into account:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525

Thanks.

-- 
Dani Moncayo

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Alan Mackenzie]

Hello, Dani.

On Wed, Feb 29, 2012 at 10:02:44AM +0100, Dani Moncayo wrote:
> On Wed, Feb 29, 2012 at 01:14, Juri Linkov <juri@jurta.org> wrote:
> >[...]
> > Regarding bug#10906, I think `c-mark-function' should be rewritten
> > to follow the logic of `mark-defun'.

Any chance of a quick summary of how c-mark-function differs from
mark-defun?

> Agreed, and BTW, if `c-mark-function' is going to be revised, please,
> take also this problem into account:

> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525

Again, what is this bug?  Could you not even quote the title line from
it?

> Thanks.

> -- 
> Dani Moncayo

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Dani Moncayo]

> Hello, Dani.

Hi Alan.

>> > Regarding bug#10906, I think `c-mark-function' should be rewritten
>> > to follow the logic of `mark-defun'.
>
> Any chance of a quick summary of how c-mark-function differs from
> mark-defun?

AFAIK, they differ at least in two things:

1. `mark-defun' saves the original point location in the mark ring,
whereas `c-mark-function' does not.  IMO, the point should be saved,
because in large defuns it may jump to a remote location and you may
want to return back to the original position.  This bug report is
about this inconsistency, as you can see in the original post.

2. Successive interactive invocations of `mark-defun' extend the
region to the next defuns (which I find useful), whereas
`c-mark-function' does not have this feature.  Bug #5525 is a request
to remove this inconsistency, as you can see in the corresponding
thread.

>> Agreed, and BTW, if `c-mark-function' is going to be revised, please,
>> take also this problem into account:
>
>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525
>
> Again, what is this bug?  Could you not even quote the title line from
> it?

I think this question is already answered.

-- 
Dani Moncayo

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Alan Mackenzie]

On Fri, Mar 02, 2012 at 12:03:48AM +0100, Dani Moncayo wrote:
> > Hello, Dani.

> Hi Alan.

> >> > Regarding bug#10906, I think `c-mark-function' should be rewritten
> >> > to follow the logic of `mark-defun'.

> > Any chance of a quick summary of how c-mark-function differs from
> > mark-defun?

> AFAIK, they differ at least in two things:

> 1. `mark-defun' saves the original point location in the mark ring,
> whereas `c-mark-function' does not.  IMO, the point should be saved,
> because in large defuns it may jump to a remote location and you may
> want to return back to the original position.  This bug report is
> about this inconsistency, as you can see in the original post.

> 2. Successive interactive invocations of `mark-defun' extend the
> region to the next defuns (which I find useful), whereas
> `c-mark-function' does not have this feature.  Bug #5525 is a request
> to remove this inconsistency, as you can see in the corresponding
> thread.

> >> Agreed, and BTW, if `c-mark-function' is going to be revised, please,
> >> take also this problem into account:

> >> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525

> > Again, what is this bug?  Could you not even quote the title line from
> > it?

> I think this question is already answered.

Thanks!

> -- 
> Dani Moncayo

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Glenn Morris]

Alan Mackenzie wrote:

>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525
>
> Again, what is this bug?  Could you not even quote the title line from
> it?

If you have rms-type issues with visiting URLs, you can do

wget 'http://debbugs.gnu.org/cgi/bugreport.cgi?mbox=yes;bug=5525'

to fetch an mbox of this or any other debbugs report, which you can then
read off-line at your convenience, eg with Rmail.

To get a listing of all bugs currently assigned to cc-mode, you can send
a mail to request@debbugs.gnu.org with "index packages cc-mode" in the
body.

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Glenn Morris]

>> Regarding bug#10906, I think `c-mark-function' should be rewritten
>> to follow the logic of `mark-defun'.
>
> Agreed, and BTW, if `c-mark-function' is going to be revised, please,
> take also this problem into account:
>
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525

As I said there, the simplest way to get consistency is surely to remove
c-mark-defun altogether, set end-of-defun-function etc appropriately,
and let mark-defun handle it.

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Alan Mackenzie]

Hello, Dani,

On Fri, Mar 02, 2012 at 12:03:48AM +0100, Dani Moncayo wrote:

> > Any chance of a quick summary of how c-mark-function differs from
> > mark-defun?

> AFAIK, they differ at least in two things:

> 1. `mark-defun' saves the original point location in the mark ring,
> whereas `c-mark-function' does not.  IMO, the point should be saved,
> because in large defuns it may jump to a remote location and you may
> want to return back to the original position.  This bug report is
> about this inconsistency, as you can see in the original post.

> 2. Successive interactive invocations of `mark-defun' extend the
> region to the next defuns (which I find useful), whereas
> `c-mark-function' does not have this feature.  Bug #5525 is a request
> to remove this inconsistency, as you can see in the corresponding
> thread.

OK, I'll amend c-mark-function to do these things.

> -- 
> Dani Moncayo

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Alan Mackenzie]

Hello, Dani.

On Fri, Mar 02, 2012 at 12:03:48AM +0100, Dani Moncayo wrote:

> >> > Regarding bug#10906, I think `c-mark-function' should be rewritten
> >> > to follow the logic of `mark-defun'.

> > Any chance of a quick summary of how c-mark-function differs from
> > mark-defun?

> AFAIK, they differ at least in two things:

> 1. `mark-defun' saves the original point location in the mark ring,
> whereas `c-mark-function' does not.  IMO, the point should be saved,
> because in large defuns it may jump to a remote location and you may
> want to return back to the original position.  This bug report is
> about this inconsistency, as you can see in the original post.

> 2. Successive interactive invocations of `mark-defun' extend the
> region to the next defuns (which I find useful), whereas
> `c-mark-function' does not have this feature.  Bug #5525 is a request
> to remove this inconsistency, as you can see in the corresponding
> thread.

> >> Agreed, and BTW, if `c-mark-function' is going to be revised, please,
> >> take also this problem into account:

> >> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525

Here is a patch which should fix these problems.  The logic around
transient-mark-mode is somewhat complicated, so I would be grateful if
you would check the patch does the Right Thing.  I haven't amended the
doc string yet.  Thanks!


diff -r f6961b0b1c51 cc-cmds.el
--- a/cc-cmds.el	Fri Mar 02 21:25:40 2012 +0000
+++ b/cc-cmds.el	Mon Mar 05 18:35:18 2012 +0000
@@ -1966,8 +1966,20 @@
 
     (if (not decl-limits)
 	(error "Cannot find any declaration")
-      (goto-char (car decl-limits))
-      (push-mark (cdr decl-limits) nil t))))
+      (let* ((extend-region-p
+	      (or (and (eq this-command 'c-mark-function)
+		       (eq last-command 'c-mark-function))))
+	     (push-mark-p (and (eq this-command 'c-mark-function)
+			       (not extend-region-p)
+			       (not (and transient-mark-mode mark-active)))))
+	(if push-mark-p (push-mark (point)))
+	(if extend-region-p
+	    (progn
+	      (exchange-point-and-mark)
+	      (goto-char (cdr (c-declaration-limits t)))
+	      (exchange-point-and-mark))
+	  (goto-char (car decl-limits))
+	  (push-mark (cdr decl-limits) nil t))))))
 
 (defun c-cpp-define-name ()
   "Return the name of the current CPP macro, or NIL if we're not in one."

> -- 
> Dani Moncayo

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Dani Moncayo]

On Mon, Mar 5, 2012 at 19:41, Alan Mackenzie <acm@muc.de> wrote:

>> > Any chance of a quick summary of how c-mark-function differs from
>> > mark-defun?
>
>> AFAIK, they differ at least in two things:
>
>> 1. `mark-defun' saves the original point location in the mark ring,
>> whereas `c-mark-function' does not.  IMO, the point should be saved,
>> because in large defuns it may jump to a remote location and you may
>> want to return back to the original position.  This bug report is
>> about this inconsistency, as you can see in the original post.
>
>> 2. Successive interactive invocations of `mark-defun' extend the
>> region to the next defuns (which I find useful), whereas
>> `c-mark-function' does not have this feature.  Bug #5525 is a request
>> to remove this inconsistency, as you can see in the corresponding
>> thread.
>
>> >> Agreed, and BTW, if `c-mark-function' is going to be revised, please,
>> >> take also this problem into account:
>
>> >> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5525
>
> Here is a patch which should fix these problems.  The logic around
> transient-mark-mode is somewhat complicated, so I would be grateful if
> you would check the patch does the Right Thing.  I haven't amended the
> doc string yet.  Thanks!

Thank you.

I've tried your patch, and it seems to work[*], at least with my usage
pattern, i.e., with `transient-mark-mode' enabled.

[*] Except for one thing:
 - If you do `C-M-h' several times in a row until the mark reaches the
end of the file, the point ends up at the end of the region, and the
mark at the beginning (which doesn't seem right).
 - If you try the same experiment with `mark-defun', the mark stays at
the end and the point at the start of the region (as expected).

-- 
Dani Moncayo

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Alan Mackenzie]

Hello, Dani.

On Mon, Mar 05, 2012 at 11:36:52PM +0100, Dani Moncayo wrote:
> On Mon, Mar 5, 2012 at 19:41, Alan Mackenzie <acm@muc.de> wrote:

> > Here is a patch which should fix these problems.  The logic around
> > transient-mark-mode is somewhat complicated, so I would be grateful if
> > you would check the patch does the Right Thing.  I haven't amended the
> > doc string yet.  Thanks!

> Thank you.

> I've tried your patch, and it seems to work[*], at least with my usage
> pattern, i.e., with `transient-mark-mode' enabled.

> [*] Except for one thing:
>  - If you do `C-M-h' several times in a row until the mark reaches the
> end of the file, the point ends up at the end of the region, and the
> mark at the beginning (which doesn't seem right).
>  - If you try the same experiment with `mark-defun', the mark stays at
> the end and the point at the start of the region (as expected).

Thanks for spotting that.

Here's a revised patch.  I think it'll work this time.



diff -r f6961b0b1c51 cc-cmds.el
--- a/cc-cmds.el	Fri Mar 02 21:25:40 2012 +0000
+++ b/cc-cmds.el	Wed Mar 07 21:06:31 2012 +0000
@@ -1950,7 +1950,12 @@
 
 (defun c-mark-function ()
   "Put mark at end of the current top-level declaration or macro, point at beginning.
-If point is not inside any then the closest following one is chosen.
+If point is not inside any then the closest following one is
+chosen.  Each successive call of this command extends the marked
+region by one function.
+
+A mark is left where the command started, unless the region is already active
+\(in Transient Mark mode).
 
 As opposed to \\[c-beginning-of-defun] and \\[c-end-of-defun], this
 function does not require the declaration to contain a brace block."
@@ -1966,8 +1971,24 @@
 
     (if (not decl-limits)
 	(error "Cannot find any declaration")
-      (goto-char (car decl-limits))
-      (push-mark (cdr decl-limits) nil t))))
+      (let* ((extend-region-p
+	      (or (and (eq this-command 'c-mark-function)
+		       (eq last-command 'c-mark-function))))
+	     (push-mark-p (and (eq this-command 'c-mark-function)
+			       (not extend-region-p)
+			       (not (and transient-mark-mode mark-active)))))
+	(if push-mark-p (push-mark (point)))
+	(if extend-region-p
+	    (progn
+	      (exchange-point-and-mark)
+	      (setq decl-limits (c-declaration-limits t))
+	      (when (not decl-limits)
+		(exchange-point-and-mark)
+		(error "Cannot find any declaration"))
+	      (goto-char (cdr decl-limits))
+	      (exchange-point-and-mark))
+	  (goto-char (car decl-limits))
+	  (push-mark (cdr decl-limits) nil t))))))
 
 (defun c-cpp-define-name ()
   "Return the name of the current CPP macro, or NIL if we're not in one."


> -- 
> Dani Moncayo

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Dani Moncayo]

> Here's a revised patch.  I think it'll work this time.

It works, thank you.

BTW, I have a question (I've still a beginner on elisp): What's the
point of the "or" form here?

> +             (or (and (eq this-command 'c-mark-function)
> +                      (eq last-command 'c-mark-function))))


It has a single argument, so that it would be equivalent to get rid of
it, like this:

> +             (and (eq this-command 'c-mark-function)
> +                      (eq last-command 'c-mark-function)))

??

-- 
Dani Moncayo

bug#10906: Bug #10906 - `c-mark-function' does set the mark well.

[Alan Mackenzie]

Bug fixed - Revision #107534.

bug#10906: 24.0.94; `c-mark-function' does set the mark well

[Alan Mackenzie]

Hello, Dani.

On Wed, Mar 07, 2012 at 10:37:10PM +0100, Dani Moncayo wrote:
> > Here's a revised patch.  I think it'll work this time.

> It works, thank you.

I've committed a patch for this into savannah.

> BTW, I have a question (I've still a beginner on elisp): What's the
> point of the "or" form here?

> > +             (or (and (eq this-command 'c-mark-function)
> > +                      (eq last-command 'c-mark-function))))


> It has a single argument, so that it would be equivalent to get rid of
> it, like this:

> > +             (and (eq this-command 'c-mark-function)
> > +                      (eq last-command 'c-mark-function)))

Yes!  That was a coding mistake, thanks for picking it up.

> ??

> -- 
> Dani Moncayo

-- 
Alan Mackenzie (Nuremberg, Germany).