Re: master 97cb255: newcomment.el (comment-line): New command on C-x C-; .

Re: master 97cb255: newcomment.el (comment-line): New command on C-x C-; .

[Stephen Berman]

This patch is missing a final `)', see below.  Please fix, and before
you push, trying compiling or at least using check-parens.  Thanks.

Steve Berman

On Sun, 08 Feb 2015 19:03:38 +0000 Artur Malabarba <bruce.connor.am@gmail.com> wrote:

> +;;;###autoload
> +(defun comment-line (n)
> +  "Comment or uncomment current line and leave point after it.
> +With positive prefix, apply to N lines including current one.
> +With negative prefix, apply to -N lines above.  Also, further
> +consecutive invocations of this command will inherit the negative
> +argument.
> +
> +If region is active, comment lines in active region instead.
> +Unlike `comment-dwim', this always comments whole lines."
> +  (interactive "p")
> +  (if (use-region-p)
> +      (comment-or-uncomment-region
> +       (save-excursion
> +         (goto-char (region-beginning))
> +         (line-beginning-position))
> +       (save-excursion
> +         (goto-char (region-end))
> +         (line-end-position)))
> +    (when (and (eq last-command 'comment-line-backward)
> +               (natnump n))
> +      (setq n (- n)))
> +    (let ((range
> +           (list (line-beginning-position)
> +                 (goto-char (line-end-position n)))))
> +      (comment-or-uncomment-region
> +       (apply #'min range)
> +       (apply #'max range)))
> +    (forward-line 1)
> +    (back-to-indentation)
> +    (unless (natnump n) (setq this-command 'comment-line-backward)))

)  ; <== here

> +
>  (provide 'newcomment)
>  
>  ;;; newcomment.el ends here

Re: master 97cb255: newcomment.el (comment-line): New command on C-x C-; .

[Artur Malabarba]

Fixed.

2015-02-08 18:07 GMT-02:00 Stephen Berman <stephen.berman@gmx.net>:
> This patch is missing a final `)', see below.  Please fix, and before
> you push, trying compiling or at least using check-parens.  Thanks.
>
> Steve Berman
>
> On Sun, 08 Feb 2015 19:03:38 +0000 Artur Malabarba <bruce.connor.am@gmail.com> wrote:
>
>> +;;;###autoload
>> +(defun comment-line (n)
>> +  "Comment or uncomment current line and leave point after it.
>> +With positive prefix, apply to N lines including current one.
>> +With negative prefix, apply to -N lines above.  Also, further
>> +consecutive invocations of this command will inherit the negative
>> +argument.
>> +
>> +If region is active, comment lines in active region instead.
>> +Unlike `comment-dwim', this always comments whole lines."
>> +  (interactive "p")
>> +  (if (use-region-p)
>> +      (comment-or-uncomment-region
>> +       (save-excursion
>> +         (goto-char (region-beginning))
>> +         (line-beginning-position))
>> +       (save-excursion
>> +         (goto-char (region-end))
>> +         (line-end-position)))
>> +    (when (and (eq last-command 'comment-line-backward)
>> +               (natnump n))
>> +      (setq n (- n)))
>> +    (let ((range
>> +           (list (line-beginning-position)
>> +                 (goto-char (line-end-position n)))))
>> +      (comment-or-uncomment-region
>> +       (apply #'min range)
>> +       (apply #'max range)))
>> +    (forward-line 1)
>> +    (back-to-indentation)
>> +    (unless (natnump n) (setq this-command 'comment-line-backward)))
>
> )  ; <== here
>
>> +
>>  (provide 'newcomment)
>>
>>  ;;; newcomment.el ends here

RE: master 97cb255: newcomment.el (comment-line): New command on C-x C-; .

[Drew Adams]

> +    (when (and (eq last-command 'comment-line-backward)
> +               (natnump n))
> +      (setq n (- n)))

It's really too bad (IMO) that you didn't take my suggestion.
It sounded like you were going to... ("Good idea. Will do.")

You did part of it - repeated upward commenting when started
with a negative prefix arg.  So far, so good.  Thx.

But this other feature, which you left out, is also useful, IMO:

  When repeated, a negative prefix arg switches direction.

If you change the above predicate to just (eq last-command
'comment-line-backward) then you get the direction-switching
that I suggested.

You then still get inheritance of a negative prefix arg when
you repeat (upward commenting instead of downward).

The direction-switching feature does not change the behavior
in any way, except if you use a negative prefix arg again,
while you are repeating.

With your implementation, hitting `C--' while you are
repeating has an effect only if you are going downward.  In
that case it does switch direction.  But if you hit `C--'
when going up then it is a no-op.  And in fact I see no way
to switch to going down again - not `C-1' or `C-u' or ...

The direction-switching behavior is thus not symmetric.
It's not very useful for it to work only when moving down,
IMO.  If someone does not want to switch direction when
repeating moving down, then s?he just won't hit `C-' again.
Since the initial prefix arg is inherited, there is no
reason to hit `C--' again, if it does nothing.  In that
context, `C--' is useless anyway.

Did you try what I suggested in this regard?  If so, did
you find something wrong with it?  To me it is handy to be
able to always reverse direction (by hitting `C--') - not
only when you are moving downward.

It is in general handy to have a quick way to reverse
the direction of an operation that is bound to a repeating
key.  One can easily repeat quickly and go past a target
position.  A quick way to reverse, while staying within
the repeating command (e.g. as opposed to using `undo'),
is an advantage in general - not just for this command.

Anyway, not a big deal.  It won't be the first time I will
have my own local version of something. ;-)  I would prefer
not to have to, of course.  And I am curious what downside
you saw to this feature, if you even tried it at all.

RE: master 97cb255: newcomment.el (comment-line): New command on C-x C-; .

[Artur Malabarba]

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

Yes, I did give it a try and it was a conscious decision, but let me start
by saying I didn't put a huge amount of thought into it.

There were three factors that made me choose this:

1. Reversing direction while going up can also be done by just hitting some
harmless key like C-a (for this particular command, C-a is harmless). That
takes the same number of keys as C-- and has the same effect. In some cases
(when you're moving one line at a time), `undo' will also do what you want
with even less keys.

2. A good portion of users won't expect it to behave like that (even if
it's written in the doc). This also applies to the current version, but see
below.

3. Unlike the current "persistent negative argument effect", this "reverse
direction twice" functionality seems like it would come up very rarely,
probably less than the number of times it would catch someone by surprise.

Combine 3 with the fact that it has perfectly plausible alternatives for
those instances when it does come up, and that's I why I decided this way.

As I said, I just thought of this very briefly. Items 2 and 3 are really
just hunches.

Cheers

On 9 Feb 2015 01:13, "Drew Adams" <drew.adams@oracle.com> wrote:
>
> > +    (when (and (eq last-command 'comment-line-backward)
> > +               (natnump n))
> > +      (setq n (- n)))
>
> It's really too bad (IMO) that you didn't take my suggestion.
> It sounded like you were going to... ("Good idea. Will do.")
>
> You did part of it - repeated upward commenting when started
> with a negative prefix arg.  So far, so good.  Thx.
>
> But this other feature, which you left out, is also useful, IMO:
>
>   When repeated, a negative prefix arg switches direction.
>
> If you change the above predicate to just (eq last-command
> 'comment-line-backward) then you get the direction-switching
> that I suggested.
>
> You then still get inheritance of a negative prefix arg when
> you repeat (upward commenting instead of downward).
>
> The direction-switching feature does not change the behavior
> in any way, except if you use a negative prefix arg again,
> while you are repeating.
>
> With your implementation, hitting `C--' while you are
> repeating has an effect only if you are going downward.  In
> that case it does switch direction.  But if you hit `C--'
> when going up then it is a no-op.  And in fact I see no way
> to switch to going down again - not `C-1' or `C-u' or ...
>
> The direction-switching behavior is thus not symmetric.
> It's not very useful for it to work only when moving down,
> IMO.  If someone does not want to switch direction when
> repeating moving down, then s?he just won't hit `C-' again.
> Since the initial prefix arg is inherited, there is no
> reason to hit `C--' again, if it does nothing.  In that
> context, `C--' is useless anyway.
>
> Did you try what I suggested in this regard?  If so, did
> you find something wrong with it?  To me it is handy to be
> able to always reverse direction (by hitting `C--') - not
> only when you are moving downward.
>
> It is in general handy to have a quick way to reverse
> the direction of an operation that is bound to a repeating
> key.  One can easily repeat quickly and go past a target
> position.  A quick way to reverse, while staying within
> the repeating command (e.g. as opposed to using `undo'),
> is an advantage in general - not just for this command.
>
> Anyway, not a big deal.  It won't be the first time I will
> have my own local version of something. ;-)  I would prefer
> not to have to, of course.  And I am curious what downside
> you saw to this feature, if you even tried it at all.

[-- Attachment #2: Type: text/html, Size: 4460 bytes --]

RE: master 97cb255: newcomment.el (comment-line): New command on C-x C-; .

[Drew Adams]

> Yes, I did give it a try and it was a conscious decision, but let me start by saying I didn't put a huge amount of thought into it. 
> There were three factors that made me choose this:
> 1. Reversing direction while going up can also be done by just hitting some harmless key like C-a (for this particular command, C-a is harmless). That takes the same number of keys as C-- and has the same effect. In some cases (when you're moving one line at a time), `undo' will also do what you want with even less keys. 
> 2. A good portion of users won't expect it to behave like that (even if it's written in the doc). This also applies to the current version, but see below. 
> 3. Unlike the current "persistent negative argument effect", this "reverse direction twice" functionality seems like it would come up very rarely, probably less than the number of times it would catch someone by surprise. 
> Combine 3 with the fact that it has perfectly plausible alternatives for those instances when it does come up, and that's I why I decided this way. 
> As I said, I just thought of this very briefly. Items 2 and 3 are really just hunches.

Like I said:

>> It won't be the first time I will have my own local version of
>> something.