* 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 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: 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