unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#10906: 24.0.94; `c-mark-function' does set the mark well
@ 2012-02-28 12:36 Dani Moncayo
  2012-02-29  9:02 ` Dani Moncayo
  2012-03-08 11:51 ` bug#10906: Bug #10906 - " Alan Mackenzie
  0 siblings, 2 replies; 14+ messages in thread
From: Dani Moncayo @ 2012-02-28 12:36 UTC (permalink / raw)
  To: 10906

[-- 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-02-28 12:36 bug#10906: 24.0.94; `c-mark-function' does set the mark well Dani Moncayo
@ 2012-02-29  9:02 ` Dani Moncayo
  2012-03-01 21:24   ` Alan Mackenzie
  2012-03-01 23:19   ` Glenn Morris
  2012-03-08 11:51 ` bug#10906: Bug #10906 - " Alan Mackenzie
  1 sibling, 2 replies; 14+ messages in thread
From: Dani Moncayo @ 2012-02-29  9:02 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 10906

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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-02-29  9:02 ` Dani Moncayo
@ 2012-03-01 21:24   ` Alan Mackenzie
  2012-03-01 23:03     ` Dani Moncayo
  2012-03-01 23:16     ` Glenn Morris
  2012-03-01 23:19   ` Glenn Morris
  1 sibling, 2 replies; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-01 21:24 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906

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).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-01 21:24   ` Alan Mackenzie
@ 2012-03-01 23:03     ` Dani Moncayo
  2012-03-01 23:13       ` Alan Mackenzie
                         ` (2 more replies)
  2012-03-01 23:16     ` Glenn Morris
  1 sibling, 3 replies; 14+ messages in thread
From: Dani Moncayo @ 2012-03-01 23:03 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 10906

> 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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-01 23:03     ` Dani Moncayo
@ 2012-03-01 23:13       ` Alan Mackenzie
  2012-03-03 14:16       ` Alan Mackenzie
  2012-03-05 18:41       ` Alan Mackenzie
  2 siblings, 0 replies; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-01 23:13 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906

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).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-01 21:24   ` Alan Mackenzie
  2012-03-01 23:03     ` Dani Moncayo
@ 2012-03-01 23:16     ` Glenn Morris
  1 sibling, 0 replies; 14+ messages in thread
From: Glenn Morris @ 2012-03-01 23:16 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 10906

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.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-02-29  9:02 ` Dani Moncayo
  2012-03-01 21:24   ` Alan Mackenzie
@ 2012-03-01 23:19   ` Glenn Morris
  1 sibling, 0 replies; 14+ messages in thread
From: Glenn Morris @ 2012-03-01 23:19 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906


>> 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.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-01 23:03     ` Dani Moncayo
  2012-03-01 23:13       ` Alan Mackenzie
@ 2012-03-03 14:16       ` Alan Mackenzie
  2012-03-05 18:41       ` Alan Mackenzie
  2 siblings, 0 replies; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-03 14:16 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906

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).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-01 23:03     ` Dani Moncayo
  2012-03-01 23:13       ` Alan Mackenzie
  2012-03-03 14:16       ` Alan Mackenzie
@ 2012-03-05 18:41       ` Alan Mackenzie
  2012-03-05 22:36         ` Dani Moncayo
  2 siblings, 1 reply; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-05 18:41 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906

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).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-05 18:41       ` Alan Mackenzie
@ 2012-03-05 22:36         ` Dani Moncayo
  2012-03-07 21:09           ` Alan Mackenzie
  0 siblings, 1 reply; 14+ messages in thread
From: Dani Moncayo @ 2012-03-05 22:36 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 10906

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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-05 22:36         ` Dani Moncayo
@ 2012-03-07 21:09           ` Alan Mackenzie
  2012-03-07 21:37             ` Dani Moncayo
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-07 21:09 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906

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).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-07 21:09           ` Alan Mackenzie
@ 2012-03-07 21:37             ` Dani Moncayo
  2012-03-09 16:49               ` Alan Mackenzie
  0 siblings, 1 reply; 14+ messages in thread
From: Dani Moncayo @ 2012-03-07 21:37 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 10906

> 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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: Bug #10906 - `c-mark-function' does set the mark well.
  2012-02-28 12:36 bug#10906: 24.0.94; `c-mark-function' does set the mark well Dani Moncayo
  2012-02-29  9:02 ` Dani Moncayo
@ 2012-03-08 11:51 ` Alan Mackenzie
  1 sibling, 0 replies; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-08 11:51 UTC (permalink / raw)
  To: 10906-done

Bug fixed - Revision #107534.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#10906: 24.0.94; `c-mark-function' does set the mark well
  2012-03-07 21:37             ` Dani Moncayo
@ 2012-03-09 16:49               ` Alan Mackenzie
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Mackenzie @ 2012-03-09 16:49 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 10906

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).





^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-03-09 16:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-28 12:36 bug#10906: 24.0.94; `c-mark-function' does set the mark well Dani Moncayo
2012-02-29  9:02 ` Dani Moncayo
2012-03-01 21:24   ` Alan Mackenzie
2012-03-01 23:03     ` Dani Moncayo
2012-03-01 23:13       ` Alan Mackenzie
2012-03-03 14:16       ` Alan Mackenzie
2012-03-05 18:41       ` Alan Mackenzie
2012-03-05 22:36         ` Dani Moncayo
2012-03-07 21:09           ` Alan Mackenzie
2012-03-07 21:37             ` Dani Moncayo
2012-03-09 16:49               ` Alan Mackenzie
2012-03-01 23:16     ` Glenn Morris
2012-03-01 23:19   ` Glenn Morris
2012-03-08 11:51 ` bug#10906: Bug #10906 - " Alan Mackenzie

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).