kill-region defadvice

kill-region defadvice

[Joe]

Hey all,

I liked the tip in Steve Yegge's "Effective Emacs" of rebinding
backward-kill-word to C-w but I still wanted to use C-w for kill-
region.  Originally I did this by writing a new function that checked
if the mark was active. However, I think that using defadvice is a
better solution but I got stuck while trying to write it.  Here's my
attempt:

(defadvice kill-region (around smart-kill activate)
  "If something is highlighted, then kill the region, otherwise
backward-kill-word"
  (interactive (list (point) (mark)))
   (if mark-active
       ad-do-it
     (backward-kill-word 1)))

When I run the command it works normally if I have something
highlighted, but if I don't have anything highlighted, I get the usual
"The mark is not active now.".

How can I kill the region if the mark is active and backward-kill-word
if its not?

Thanks for the help,
Joe

Re: kill-region defadvice

[Nikolaj Schumacher]

Joe <joesmoe10@gmail.com> wrote:

> Originally I did this by writing a new function that checked
> if the mark was active. However, I think that using defadvice is a
> better solution but I got stuck while trying to write it.

No, it's not!  By using advice, you modify the kill-region function and
risk that any command using it will backward-kill-word instead.

> When I run the command it works normally if I have something
> highlighted, but if I don't have anything highlighted, I get the usual
> "The mark is not active now.".

The reason for this is: (mark)
That throws the error if the mark is not active.

Also, try setting the variable debug-on-error to t.  It'll give you a
backtrace to help locate such errors.

> How can I kill the region if the mark is active and backward-kill-word
> if its not?

(defun control-w-dwim ()
  (interactive)
  (if mark-active
      (call-interactively 'kill-region)
    (call-interactively 'backward-kill-word)))


regards,
Nikolaj Schumacher

Re: kill-region defadvice

[Kevin Rodgers]

Nikolaj Schumacher wrote:
> Joe <joesmoe10@gmail.com> wrote:
> 
>> Originally I did this by writing a new function that checked
>> if the mark was active. However, I think that using defadvice is a
>> better solution but I got stuck while trying to write it.
> 
> No, it's not!  By using advice, you modify the kill-region function and
> risk that any command using it will backward-kill-word instead.

Just write the advice as

    (if (and (interactive-p) mark-active)
        ad-do-it
      (backward-kill-word 1))

-- 
Kevin Rodgers
Denver, Colorado, USA

Re: kill-region defadvice

[Nikolaj Schumacher]

Kevin Rodgers <kevin.d.rodgers@gmail.com> wrote:

> Nikolaj Schumacher wrote:
>> Joe <joesmoe10@gmail.com> wrote:
>>
>>> Originally I did this by writing a new function that checked
>>> if the mark was active. However, I think that using defadvice is a
>>> better solution but I got stuck while trying to write it.
>>
>> No, it's not!  By using advice, you modify the kill-region function and
>> risk that any command using it will backward-kill-word instead.
>
> Just write the advice as
>
>    (if (and (interactive-p) mark-active)
>        ad-do-it
>      (backward-kill-word 1))

Still not a good idea.
`interactive-p' will return nil when used in a keyboard macro.  That
will most likely cause unintended behavior at some point.


regards,
Nikolaj Schumacher

Re: kill-region defadvice

[Kevin Rodgers]

Nikolaj Schumacher wrote:
> Kevin Rodgers <kevin.d.rodgers@gmail.com> wrote:
> 
>> Nikolaj Schumacher wrote:
>>> Joe <joesmoe10@gmail.com> wrote:
>>>
>>>> Originally I did this by writing a new function that checked
>>>> if the mark was active. However, I think that using defadvice is a
>>>> better solution but I got stuck while trying to write it.
>>> No, it's not!  By using advice, you modify the kill-region function and
>>> risk that any command using it will backward-kill-word instead.
>> Just write the advice as
>>
>>    (if (and (interactive-p) mark-active)
>>        ad-do-it
>>      (backward-kill-word 1))
> 
> Still not a good idea.
> `interactive-p' will return nil when used in a keyboard macro.  That
> will most likely cause unintended behavior at some point.

(if (and (or (interactive-p) executing-kbd-macro)
	 mark-active)
...

-- 
Kevin Rodgers
Denver, Colorado, USA

Re: kill-region defadvice

[Nikolaj Schumacher]

Kevin Rodgers <kevin.d.rodgers@gmail.com> wrote:

> Nikolaj Schumacher wrote:
>> Kevin Rodgers <kevin.d.rodgers@gmail.com> wrote:
>>
>>> Nikolaj Schumacher wrote:
>>>> Joe <joesmoe10@gmail.com> wrote:
>>>>
>>>>> Originally I did this by writing a new function that checked
>>>>> if the mark was active. However, I think that using defadvice is a
>>>>> better solution but I got stuck while trying to write it.
>>>>
>>>> No, it's not!  By using advice, you modify the kill-region function and
>>>> risk that any command using it will backward-kill-word instead.
>>>
>>> Just write the advice as
>>>
>>>    (if (and (interactive-p) mark-active)
>>>        ad-do-it
>>>      (backward-kill-word 1))
>>
>> Still not a good idea.
>> `interactive-p' will return nil when used in a keyboard macro.  That
>> will most likely cause unintended behavior at some point.
>
> (if (and (or (interactive-p) executing-kbd-macro)
> 	 mark-active)

First of all, that should probably read:

(if (or (not (or (interactive-p) executing-kbd-macro)
             mark-active))

But that still won't work.  executing-kbd-macro will remain non-nil when
called non-interactively by another command while inside a macro.

Of course, I'm not saying that it can't be done.  I'm just saying it's
easy to mess up and difficult to debug.


regards,
Nikolaj Schumacher

Re: kill-region defadvice

[Barry Margolin]

In article 
<c09cac8e-fd66-4731-947d-c18a9060ba92@d1g2000hsg.googlegroups.com>,
 Joe <joesmoe10@gmail.com> wrote:

> Hey all,
> 
> I liked the tip in Steve Yegge's "Effective Emacs" of rebinding
> backward-kill-word to C-w but I still wanted to use C-w for kill-
> region.  Originally I did this by writing a new function that checked
> if the mark was active. However, I think that using defadvice is a
> better solution but I got stuck while trying to write it.

Others have answered your programming question, but I'd like to comment 
on your original premise.

I assume you only want this special behavior when the command is bound 
to C-w, because the "w" means "word" to you (was this the kill-word 
command in some other editor you've used in the past?).  But if you use 
defadvice, your advice will be obeyed even if the command were on some 
other key.  Or if a function used (call-interactively 'kill-region).

So I think it would be better to write a new function, and bind C-w to 
that.

-- 
Barry Margolin, barmar@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***
*** PLEASE don't copy me on replies, I'll read them in the group ***

Re: kill-region defadvice

[Joe]

On Jun 8, 5:18 pm, Barry Margolin <barmar@alum.mit.edu> wrote:

> I assume you only want this special behavior when the command is bound
> to C-w, because the "w" means "word" to you (was this the kill-word
> command in some other editor you've used in the past?).  But if you use
> defadvice, your advice will be obeyed even if the command were on some
> other key.  Or if a function used (call-interactively 'kill-region).
>
> So I think it would be better to write a new function, and bind C-w to
> that.


I like C-w because it's easy to type and works in a bash shell.

Thanks for all the tips, I understand the potential usability
problems, so I think I'll stick with writing an extra function and
binding that to C-w.

Cheers,
Joe