Various simple.el patches

Various simple.el patches

[Stefan Monnier]

1 - When killing the same thing over and over again, don't fill
    the kill-ring unnecessarily.

@@ -1810,6 +1859,7 @@
 argument is not used by `insert-for-yank'.  However, since Lisp code
 may access and use elements from the kill-ring directly, the STRING
 argument should still be a \"useful\" string for such uses."
+  (when (equal string (car kill-ring)) (setq replace t))
   (if (> (length string) 0)
       (if yank-handler
 	  (put-text-property 0 1 'yank-handler yank-handler string)


2a - When yanking with an active region, do `delete-selection'.
2b - And if the yanked text is the same as the current region,
     then yank the previous text.  This way you can mouse-select
     one piece of text (which pushes it on the kill-ring),
     then mouse-select another, then `yank' which replaces the
     second piece of text with the first one.

@@ -2067,7 +2117,11 @@
   ;; If we don't get all the way thru, make last-command indicate that
   ;; for the following command.
   (setq this-command t)
-  (push-mark (point))
+  (if (and transient-mark-mode mark-active)
+      (let ((old (delete-and-extract-region (region-beginning) (region-end))))
+	(if (and (listp arg) (string= old (current-kill 0)))
+	    (rotate-yank-pointer 1)))
+    (push-mark (point)))
   (insert-for-yank (current-kill (cond
 				  ((listp arg) 0)
 				  ((eq arg '-) -1)

3 - Make C-k at EOB kill the last line.
    This is mostly useful for the minibuffer and maybe it should
    only do so in the minibuffer.

@@ -2196,7 +2250,13 @@
 		 (if arg
 		     (forward-visible-line (prefix-numeric-value arg))
 		   (if (eobp)
-		       (signal 'end-of-buffer nil))
+		       ;; Make C-k at end of minibuf do C-a C-k.
+		       (if (eq last-command this-command)
+			   ;; Don't do it repeatedly: the user might
+			   ;; stupidly be pressing C-k to delete-to-eob (he
+			   ;; should do M-> C-w instead, of course).
+			   (signal 'end-of-buffer nil)
+			 (beginning-of-line)))
 		   (let ((end
 			  (save-excursion
 			    (end-of-visible-line) (point))))


-- Stefan

Re: Various simple.el patches

[Kai Großjohann]

Good stuff!  Just my 2 cents.
-- 
This line is not blank.

Re: Various simple.el patches

[Miles Bader]

Actually I think the ^K hack is quite disgusting...

-Miles

-- 
`There are more things in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.'

Re: Various simple.el patches

[Kai Großjohann]

Miles Bader <miles@gnu.org> writes:

> Actually I think the ^K hack is quite disgusting...

Do you dislike the function or its implementation?
-- 
This line is not blank.

Re: Various simple.el patches

[Miles Bader]

On Wed, May 14, 2003 at 03:58:10PM +0200, Kai Gro?johann wrote:
> > Actually I think the ^K hack is quite disgusting...
> 
> Do you dislike the function or its implementation?

The function.

The idea that `oh, in _this_ situation, it works _backwards_,' not because it
actually makes much sense, but (apparently) simply because that particular
case is otherwise a no-op seems like a quick trip to a totally confusing user
interface.  If the conventional two-key idiom is really so inconvenient, why
not bind another key to it (personally I have C-u do that in the minibuffer,
but I can see why most people wouldn't like this)?

-Miles
-- 
`The suburb is an obsolete and contradictory form of human settlement'

Re: Various simple.el patches

[Kai Großjohann]

Miles Bader <miles@gnu.org> writes:

> The idea that `oh, in _this_ situation, it works _backwards_,' not because it
> actually makes much sense, but (apparently) simply because that particular
> case is otherwise a no-op seems like a quick trip to a totally confusing user
> interface.

Well, err.  I agree, too much dwimishness is dangerous.  There was
precedent with C-t, though.

I think I'll keep my mouth shut, now.  I mean, I have S-<backspace>
bound to kai-kill-whole-line, anyway...

What do people think about a kill-whole-line function that does like
C-a C-k C-k?  I think it might be useful.
-- 
This line is not blank.

Re: Various simple.el patches

[Ami Fischman]

Miles Bader <miles@gnu.org> writes:

> The idea that `oh, in _this_ situation, it works _backwards_,' not because it
> actually makes much sense, but (apparently) simply because that particular
> case is otherwise a no-op seems like a quick trip to a totally confusing user
> interface.

Not to mention that if point is 10 lines above the EOB, and user wants to
kill all remaining text, user can now just hold down ^K until emacs beeps,
but in the new system, that'll devour everything in the buffer (_above_ the
original point, as well).  
I think.

-- 
  Ami Fischman
  usenet@fischman.org

Re: Various simple.el patches

[Stefan Monnier]

> Not to mention that if point is 10 lines above the EOB, and user wants to
> kill all remaining text, user can now just hold down ^K until emacs beeps,
> but in the new system, that'll devour everything in the buffer (_above_ the
> original point, as well).
> I think.

You think wrong, because I thought of it before you did ;-)


	Stefan

Re: Various simple.el patches

[Jan Nieuwenhuizen]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

> 1 - When killing the same thing over and over again, don't fill
>     the kill-ring unnecessarily.

Don't do this.  It will unexpectedly break moving stuff like:

   foo
   bar
   bar
   baz

using kill (esp with kill-whole-line t) and subsequent yank.

Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: Various simple.el patches

[Stefan Monnier]

> > 1 - When killing the same thing over and over again, don't fill
> >     the kill-ring unnecessarily.
> 
> Don't do this.  It will unexpectedly break moving stuff like:
> 
>    foo
>    bar
>    bar
>    baz
> 
> using kill (esp with kill-whole-line t) and subsequent yank.

No, this is handled somewhere else.  The only negative effect
it has (AFAIK) is if you do C-k C-y C-k C-k C-k C-k on the above
example: The second C-k overwrites the data from the first
so if you do C-y M-y you won't get "foo" but you'll get
whatever was on the kill-ring before.


	Stefan

Re: Various simple.el patches

[Richard Stallman]

    What do people think about a kill-whole-line function that does like
    C-a C-k C-k?  I think it might be useful.

I have nothing against adding such a function.  In fact, I think
we once had one somewhere.   There is no room to give it a key binding
by default.

Re: Various simple.el patches

[Richard Stallman]

    The idea that `oh, in _this_ situation, it works _backwards_,' not because it
    actually makes much sense, but (apparently) simply because that particular
    case is otherwise a no-op seems like a quick trip to a totally confusing user
    interface.

I agree.  I won't accept this change.

Re: Various simple.el patches

[Kai Großjohann]

Richard Stallman <rms@gnu.org> writes:

>     What do people think about a kill-whole-line function that does like
>     C-a C-k C-k?  I think it might be useful.
>
> I have nothing against adding such a function.  In fact, I think
> we once had one somewhere.   There is no room to give it a key binding
> by default.

What about S-<backspace>?  That keybinding is not available when
running on a tty, but AFAIK it it not bound by default.  And CUA
doesn't bind it either, I think.
-- 
This line is not blank.

Re: Various simple.el patches

[Richard Stallman]

    1 - When killing the same thing over and over again, don't fill
	the kill-ring unnecessarily.

    ...
    No, this is handled somewhere else.  The only negative effect
    it has (AFAIK) is if you do C-k C-y C-k C-k C-k C-k on the above
    example: The second C-k overwrites the data from the first
    so if you do C-y M-y you won't get "foo" but you'll get
    whatever was on the kill-ring before.

That problem could be solved by having a flag that says there
are notionally two copies of the string on the kill ring.
If you append more killed text when the flag is set, it should
copy the string first, so that the original string remains in the
kill ring.

    2a - When yanking with an active region, do `delete-selection'.

This would be worth trying out, with a variable to control it
and disabled by default.

    3 - Make C-k at EOB kill the last line.

I agree with those who have criticized this.

Re: Various simple.el patches

[Stefan Monnier]

>     1 - When killing the same thing over and over again, don't fill
> 	the kill-ring unnecessarily.
> 
>     ...
>     No, this is handled somewhere else.  The only negative effect
>     it has (AFAIK) is if you do C-k C-y C-k C-k C-k C-k on the above
>     example: The second C-k overwrites the data from the first
>     so if you do C-y M-y you won't get "foo" but you'll get
>     whatever was on the kill-ring before.
> 
> That problem could be solved by having a flag that says there
> are notionally two copies of the string on the kill ring.
> If you append more killed text when the flag is set, it should
> copy the string first, so that the original string remains in the
> kill ring.

I could try something like that if the minor loss
turns out to be a problem.

>     2a - When yanking with an active region, do `delete-selection'.
> This would be worth trying out, with a variable to control it
> and disabled by default.

I think it's too minor to deserve a config var.  If it can't be
enabled by default, it might as well not exist.

What about the `2b' part (i.e. if the kill-ring says "foo" and the region
is active and contains "foo", delete the region and replace it with
the second element of the kill-ring) ?


	Stefan

Re: Various simple.el patches

[Richard Stallman]

    What about S-<backspace>?

I guess I have no objection.

Re: Various simple.el patches

[Richard Stallman]

    > That problem could be solved by having a flag that says there
    > are notionally two copies of the string on the kill ring.
    > If you append more killed text when the flag is set, it should
    > copy the string first, so that the original string remains in the
    > kill ring.

    I could try something like that if the minor loss
    turns out to be a problem.

We should not knowingly introduce an ugliness when it
is so easy to avoid it.  So if you'd like to install such a change,
please do the extra work now, to make that change not be ugly.

    >     2a - When yanking with an active region, do `delete-selection'.
    > This would be worth trying out, with a variable to control it
    > and disabled by default.

    I think it's too minor to deserve a config var.  If it can't be
    enabled by default, it might as well not exist.

If you install it with a variable to control it,
we can try it out and see what people think of it.

    What about the `2b' part (i.e. if the kill-ring says "foo" and the region
    is active and contains "foo", delete the region and replace it with
    the second element of the kill-ring) ?

That seems too weird to me.

Re: Various simple.el patches

[Kai Großjohann]

Richard Stallman <rms@gnu.org> writes:

>     What about the `2b' part (i.e. if the kill-ring says "foo" and the region
>     is active and contains "foo", delete the region and replace it with
>     the second element of the kill-ring) ?
>
> That seems too weird to me.

But it is something that people used to CUA systems might like.
There a normal procedure is this: you mark some text, hit Ctrl-C for
copy.  Then you mark some other text, hit Ctrl-V for paste.  This
replaces the other text with the previously copied text.

IIUC, Stefan is suggesting a similar behavior.  The only difference
is that Emacs sometimes implicitly invokes copy (when you mark
something with the mouse), so it looks weird when described.
-- 
This line is not blank.

Re: Various simple.el patches

[Kai Großjohann]

Richard Stallman <rms@gnu.org> writes:

>     What about S-<backspace>?
>
> I guess I have no objection.

Good :-)  Here is a proposed patch.  I'm not sure if I did it right.
Somehow I get the feeling that using kill-line was not the right
thing to do.  I'm just so lazy...

cvs server: Diffing lisp
Index: lisp/bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.116
diff -u -r1.116 bindings.el
--- lisp/bindings.el	13 Apr 2003 06:30:59 -0000	1.116
+++ lisp/bindings.el	18 May 2003 15:38:24 -0000
@@ -743,6 +743,7 @@
 ;(define-key global-map [delete] 'backward-delete-char)
 
 ;; natural bindings for terminal keycaps --- defined in X keysym order
+(define-key global-map [S-backspace]    'kill-whole-line)
 (define-key global-map [home]		'beginning-of-line)
 (define-key global-map [C-home]		'beginning-of-buffer)
 (define-key global-map [M-home]		'beginning-of-buffer-other-window)
Index: lisp/simple.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/simple.el,v
retrieving revision 1.602
diff -u -r1.602 simple.el
--- lisp/simple.el	16 May 2003 21:17:52 -0000	1.602
+++ lisp/simple.el	18 May 2003 15:38:35 -0000
@@ -2205,6 +2205,22 @@
 		       (goto-char end))))
 		 (point))))
 
+(defun kill-whole-line (&optional arg)
+  "Kill current line.
+With prefix arg, kill that many lines from point.
+If arg is negative, kill backwards.
+If arg is zero, kill current line but exclude the trailing newline."
+  (interactive "P")
+  (setq arg (prefix-numeric-value arg))
+  (cond ((zerop arg)
+	 (kill-region (progn (forward-visible-line 0) (point))
+		      (progn (end-of-visible-line) (point))))
+	((< arg 0)
+	 (forward-visible-line 1)
+	 (kill-line arg))
+	(t
+	 (forward-visible-line 0)
+	 (kill-line arg))))
 
 (defun forward-visible-line (arg)
   "Move forward by ARG lines, ignoring currently invisible newlines only.
cvs server: Diffing lisp/calc
cvs server: Diffing lisp/calendar
cvs server: Diffing lisp/emacs-lisp
cvs server: Diffing lisp/emulation
cvs server: Diffing lisp/eshell
cvs server: Diffing lisp/gnus
cvs server: Diffing lisp/international
cvs server: Diffing lisp/language
cvs server: Diffing lisp/mail
cvs server: Diffing lisp/mh-e
cvs server: Diffing lisp/net
cvs server: Diffing lisp/obsolete
cvs server: Diffing lisp/play
cvs server: Diffing lisp/progmodes
cvs server: Diffing lisp/term
cvs server: Diffing lisp/textmodes
cvs server: Diffing lisp/toolbar


-- 
This line is not blank.

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohann wrote:

   Good :-)  Here is a proposed patch.  I'm not sure if I did it right.
   Somehow I get the feeling that using kill-line was not the right
   thing to do.

The use of kill-line does not bother me personally, but in terms of
the functionality there are two lines I would like to add to the code.
More precisely:

     ((< arg 0)
      (forward-visible-line 1)
      (kill-line arg)
      (unless (bobp) (forward-visible-line -1)))

The reasons for adding the last line are that it looks more intuitive
for point to move backward while killing backward, that this way there
is a differentiation between numeric arguments of 1 and -1, and most
importantly that one can do M--1 S-<backspace> C-x z z z z...

	    (t
	     (forward-visible-line 0)
	     (if (eobp)
	         (signal 'end-of-buffer nil)
               (kill-line arg)))))

Because you call kill-line with an argument, there is, unlike for C-k,
no warning at the end of the buffer.  I have personally
default-indicate-empty-lines set to t, but with the default value of
nil, the warning at the end of the buffer is really essential while
killing invisible empty lines at the end of the buffer.  Otherwise,
one does not know when to stop.

With the two added lines the function becomes:

(defun kill-whole-line (&optional arg)
  "Kill current line.
With prefix arg, kill that many lines from point.
If arg is negative, kill backwards.
If arg is zero, kill current line but exclude the trailing newline."
  (interactive "P")
  (setq arg (prefix-numeric-value arg))
  (cond ((zerop arg)
	 (kill-region (progn (forward-visible-line 0) (point))
		      (progn (end-of-visible-line) (point))))
	((< arg 0)
	 (forward-visible-line 1)
	 (kill-line arg)
	 (unless (bobp) (forward-visible-line -1)))
	(t
	 (forward-visible-line 0)
	 (if (eobp)
	     (signal 'end-of-buffer nil)
	   (kill-line arg)))))

Sincerely,

Luc.

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> With the two added lines the function becomes:

Thanks, Luc.  Committing...
-- 
This line is not blank.

Re: Various simple.el patches

[Luc Teirlinck]

There are still problems with the function, both in the original
version and after my changes.  Remember that successive kill commands,
no matter what they are, should handle the kill ring correctly.

Start out with:

word1 word2 word3 word4

and point between word3 and word4.

Do M-<backspace> S-<backspace> C-y.

Not the desired result.

I believe that with point in the middle of the to be killed region,
one should use two kill commands: one forward kill and one backward
kill.  Otherwise, the kill ring is bound to be messed up.

Sincerely,

Luc.

Re: Various simple.el patches

[Richard Stallman]

    >     What about the `2b' part (i.e. if the kill-ring says "foo" and the region
    >     is active and contains "foo", delete the region and replace it with
    >     the second element of the kill-ring) ?
    >
    > That seems too weird to me.

    But it is something that people used to CUA systems might like.

Perhaps implement it in CUA mode.

Re: Various simple.el patches

[Luc Teirlinck]

There are still problems with the present version of `kill-whole-line'.
Start out with:

word1 word2

and place point between word1 and word2.
Now S-<backspace> C-y yields:
 word2 (followed by a newline)
M-0 S-<backspace> C-y yields:
 word2 (without newline)
and M--1 S-<backspace> C-y yields:
word1 (without trailing space or newline)

Makes no sense.

The problem is that kill-region sets this-command to 'kill-region, but
only when the command returns to the command loop will this result in
last-command being set to 'kill-region and kill-region checks
last-command to figure out whether to append or not.  So we have to
bind last-command to the appropriate value explicitly.  The:

     ((< arg 0)
      (forward-visible-line 1)

which I suggested to add has to be removed, since it moves point
without killing, potentially messing up the kill ring.  There goes the
M-- S-<backspace> C-x z z z z fun.  Not the end of the problems.
Things need to work out OK in read-only buffers.  But there killed
text does not really get removed and, as a result, it winds up getting
copied twice in certain situations.  Hence the need for
save-excursion's.  After all of this we wind up with version 1 of
kill-whole-line below.

That version works more or less OK, but some things seem less than
ideal:  

1.  As already mentioned, the possibility of killing backward with M--
    S-<backspace> C-x z z z is gone.

2.  The now more complex function gets very vulnerable to any possible
    future changes (including possible customization variables) in the
    not very primitive function kill-line.  So on second thought, it
    might actually be better to write the function entirely in terms
    of kill-region after all.

3.  Killing in read-only buffers turns out not to be either terribly
    convenient or terribly intuitive.

Version 2 of the function (see below) seems to take care of all three
problems.  Maybe somewhat strange at first is that S-<backspace> with
a negative argument leaves point at the *end* of the preceding non
killed line.  But M-0 S-<backspace> already leaves point at the end of
a line in read-only buffers, even in version 1.  I believe that the
behavior of version 2 can easily be understood, by the user, in the
following terms: with zero argument, point first moves to the
beginning of the line and then the line gets killed without killing
the newline.  With positive argument, point also first moves to the
beginning of the line and then a number of lines and the following
newline are killed.  With negative argument, point first moves to the
end of the line and the killing ends with the kill of the last
*preceding* newline.  This way, the resulting behavior seems intuitive,
even in read-only buffers.  We can not really implement the function
in this straightforward way, because that could severely mess up the
kill ring.  (This is what makes the actual implementation somewhat
complicated.)  But except for the fact that the kill ring gets handled
correctly, the behavior is equivalent.

I include below the two versions.  They differ in behavior for
negative arguments.  Version 2 does not use kill-line.  Once we decide
on which version to use (1 or 2 or some other one), I will still do
some further checking of that version in other situations, such as in
the presence of invisible text, just to make sure.  Personally, I
prefer version 2.

Version 1:

===File ~/kwlversion1=======================================
(defun kill-whole-line (&optional arg)
  "Kill current line.
With prefix arg, kill that many lines from point.
If arg is negative, kill backwards.
If arg is zero, kill current line but exclude the trailing newline."
  (interactive "P")
  (setq arg (prefix-numeric-value arg))
  (cond ((zerop arg)
	 (save-excursion
	   (kill-region (point) (progn (forward-visible-line 0) (point))))
	 (let ((last-command
		(if (eq this-command 'kill-region)
		    'kill-region
		  last-command)))
	   (kill-region (point) (progn (end-of-visible-line) (point)))))
	((< arg 0)
	 (save-excursion
	   (kill-line 1))
	 (let ((last-command
		(if (eq this-command 'kill-region)
		    'kill-region
		  last-command)))
	   (kill-line (1+ arg))))
	(t
	 (save-excursion
	   (kill-line 0))
	 (if (eobp) (signal 'end-of-buffer nil))
	 (let ((last-command
		(if (eq this-command 'kill-region)
		    'kill-region
		  last-command)))
	   (kill-line arg)))))

============================================================

Version 2:

===File ~/kwlversion2=======================================
(defun kill-whole-line (&optional arg)
  "Kill current line.
With prefix arg, kill that many lines from point.
If arg is negative, kill backwards.
If arg is zero, kill current line but exclude the trailing newline."
  (interactive "P")
  (setq arg (prefix-numeric-value arg))
  (cond ((zerop arg)
	 (save-excursion
	   (kill-region (point) (progn (forward-visible-line 0) (point))))
	 (let ((last-command
		(if (eq this-command 'kill-region)
		    'kill-region
		  last-command)))
	   (kill-region (point) (progn (end-of-visible-line) (point)))))
	((< arg 0)
	 (save-excursion
	   (kill-region (point) (progn (end-of-visible-line) (point))))
	 (let ((last-command
		(if (eq this-command 'kill-region)
		    'kill-region
		  last-command)))
	   (kill-region (point)
			(progn (forward-visible-line (1+ arg))
			       (unless (bobp) (backward-char))
			       (point)))))
	(t
	 (if (and (bolp) (eobp)) (signal 'end-of-buffer nil))
	 (save-excursion
	   (kill-region (point) (progn (forward-visible-line 0) (point))))
	 (let ((last-command
		(if (eq this-command 'kill-region)
		    'kill-region
		  last-command)))
	   (kill-region (point)
			(progn (forward-visible-line arg) (point)))))))

============================================================

Re: Various simple.el patches

[Luc Teirlinck]

There are still problems with both versions of kill-whole-line I
proposed.  But these problems might actually be problems with
kill-region.  I will report somewhat later on these separately, since
they have nothing to do with kill-whole-line.

Sincerely,

Luc.

Re: Various simple.el patches

[Tak Ota]

Sorry if I am missing a point but I thought kill-while-line was a part
of kill-line features.

(lambda ()
  (interactive)
  (beginning-of-line)
  (let ((kill-whole-line t))
    (call-interactively 'kill-line)))

Wed, 21 May 2003 09:07:32 -0500 (CDT): Luc Teirlinck <teirllm@dms.auburn.edu> wrote:

> There are still problems with both versions of kill-whole-line I
> proposed.  But these problems might actually be problems with
> kill-region.  I will report somewhat later on these separately, since
> they have nothing to do with kill-whole-line.
> 
> Sincerely,
> 
> Luc.
> 
> 
> 
> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohann wrote:

   Sorry if I am missing a point but I thought kill-while-line was a part
   of kill-line features.

We are talking about a new function `kill-whole-line', not about the
variable of the same name. 

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

Maybe I missed the point you were trying to make.  I will take alook
at how your lambda function affects the kill ring.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

Your lambda function has the same problems with the kill ring as Kai's
original function.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

line-- 

I believe that I finally have a version of kill-whole-line that treats
the kill ring correctly regardless of whether the buffer is read-only
or not.  It is actually simpler than the (incorrect) version I sent
yesterday.  (I was making things too difficult.)  It still suffers
"externally" from the problem with kill-region I reported earlier
today.  That is, if you are at the end of the buffer, do M-2 C-k and
then a kill command, you are in trouble no matter what the kill
command is.  That is a separate problem.  However the version of the
function I gave yesterday suffered "internally" from the problem,
without prior "empty kill".  This is no longer the case.

It seems that my latest version works perfectly in the absence of
invisible newlines.  However...

There seem to be bugs with forward-visible-line.  I will report on
these separately.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

The following version of kill-whole-line should work OK with the
kill-ring, including read-only buffers, and with invisible text.
It implements the version I referred to previously as "version2", but
other types of behavior could be implemented in a similar way, should
they be deemed more desirable.

Invisible text will work OK if you apply the patch to simple.el which
I sent yesterday and avoid using lists with two or more elements as
invisibility properties.  If you do the latter, the invisibility
related machinery of simple.el will malfunction.  This is a problem
unrelated to kill-whole-line, which can easily be solved by using a
corrected version of line-move-invisible, either the one I sent today,
or one from the C code, as Stefan suggested.

===File ~/newkwlversion2.el=================================
(defun kill-whole-line (&optional arg)
  "Kill current line.
With prefix arg, kill that many lines from point.
If arg is negative, kill backwards.
If arg is zero, kill current line but exclude the trailing newline."
  (interactive "P")
  (setq arg (prefix-numeric-value arg))
  (if (and (> arg 0) (eobp) (save-excursion (forward-visible-line 0) (eobp)))
      (signal 'end-of-buffer nil))
  (if (and (< arg 0) (bobp) (save-excursion (end-of-visible-line) (bobp)))
      (signal 'beginning-of-buffer nil))
  (unless (eq last-command 'kill-region)
    (kill-new "")
    (setq last-command 'kill-region))
  (cond ((zerop arg)
	 (save-excursion
	   (kill-region (point) (progn (forward-visible-line 0) (point))))
	 (kill-region (point) (progn (end-of-visible-line) (point))))
	((< arg 0)
	 (save-excursion
	   (kill-region (point) (progn (end-of-visible-line) (point))))
	 (kill-region (point)
		      (progn (forward-visible-line (1+ arg))
			     (unless (bobp) (backward-char))
			     (point))))
	(t
	 (save-excursion
	   (kill-region (point) (progn (forward-visible-line 0) (point))))
	 (kill-region (point)
		      (progn (forward-visible-line arg) (point))))))

============================================================

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> The following version of kill-whole-line should work OK with the
> kill-ring, including read-only buffers, and with invisible text.
> It implements the version I referred to previously as "version2", but
> other types of behavior could be implemented in a similar way, should
> they be deemed more desirable.

I vote for including this code in Emacs.  Do you want to install it?

(Minor question: are the save-excursions necessary?  I don't know if
it is desirable to try to minimize them.)
-- 
This line is not blank.

Re: Various simple.el patches

[Stefan Monnier]

> The following version of kill-whole-line should work OK with the
> kill-ring, including read-only buffers, and with invisible text.

I'd really like to see it implemented as a wrapper around kill-line.
After all, it seems that the bug you're fixing also plagues kill-line
when `kill-whole-line' is set, so it should be fixed there too.


	Stefan

Re: Various simple.el patches

[Luc Teirlinck]

Stefan Monnier wrote:

   > The following version of kill-whole-line should work OK with the
   > kill-ring, including read-only buffers, and with invisible text.

   I'd really like to see it implemented as a wrapper around kill-line.
   After all, it seems that the bug you're fixing also plagues kill-line
   when `kill-whole-line' is set, so it should be fixed there too.

To make sure we understand each other: we are talking about the bug
with the kill ring handling.  I was unable to produce such bugs with
kill-line while kill-whole-line is set to t.  Can you produce such
bugs?  It is easy to actually produce such bugs while trying to
implement one's own versions of the function kill-whole-line using the
variable kill-whole-line.  But I believe that the point of the
variable is that you can do C-a C-k C-k... instead of C-a and then
double the amount of C-k's. It is clear to the user that the C-a is
going to "break" the kill-ring and start a new entry.

   I'd really like to see it implemented as a wrapper around kill-line.

This is a matter of taste.  Kai originally implemented it as such, but
also seemed to suggest that he thought it might be better to use only
kill-region.  My fix for the kill-ring handling indeed would work just
as fine for a kill-line based function.  My version of kill-whole-line
allows for a M--1 S-<backspace> C-x z z z... functionality which
sometimes can be useful.  I guess that, one way or the other, it
should be possible to get the same effect with a kill-line based
function.  

But kill-line is very much a user level function with plenty of
special features.  Use of kill-line would make `kill-whole-line'
vulnerable to any change or addition to the user level features of
kill-line.  I prefer using the more primitive kill-region.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohann wrote:

   I vote for including this code in Emacs.  Do you want to install it?

I currently have no write access to the Emacs CVS.  I may or may not
want such access depending on exactly how much CVS knowledge is
necessary to commit changes without fear of doing some horrible
stupidities.  (I currently know no more CVS than necessary to check
out the CVS from Savannah.)  I just come to write Richard about this.

   (Minor question: are the save-excursions necessary?  I don't know if
   it is desirable to try to minimize them.)

They are necessary for read-only buffers.  Otherwise, you wind up
moving over the same text twice and copying it twice to the kill ring.
It certainly should be avoided to use completely unnecessary
save-excursion's.  On the other hand, I do not believe that
use of save-excursion poses such efficiency problems that one should
go completely out of one's way to avoid it, at the expense of more
complicated code.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohan wrote:

   (Minor question: are the save-excursions necessary?  I don't know if
   it is desirable to try to minimize them.)

To be more precise than in my previous answer:

The documentation string of save-excursion says:

Save point, mark, and current buffer; execute BODY; restore those things.

In our situation, we only need to save and restore the value of
point.  I have often seen save-excursion used in such situations.

One could use the obvious:

(let ((opoint (point)))
     ...
     (goto-char opoint))

I doubt the gain in efficiency would be measurable to any degree, but
it certainly would be easy to change the code to use the above let
forms instead of save-excursion.

Sincerely,

Luc.

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Kai Grossjohann wrote:
>
>    I vote for including this code in Emacs.  Do you want to install it?
>
> I currently have no write access to the Emacs CVS.  I may or may not
> want such access depending on exactly how much CVS knowledge is
> necessary to commit changes without fear of doing some horrible
> stupidities.  (I currently know no more CVS than necessary to check
> out the CVS from Savannah.)  I just come to write Richard about this.

Okay.  I'm sure someone else can/will commit it.

>    (Minor question: are the save-excursions necessary?  I don't know if
>    it is desirable to try to minimize them.)
>
> They are necessary for read-only buffers.  Otherwise, you wind up
> moving over the same text twice and copying it twice to the kill ring.

Ah!  Okay, then.  As you say, it is much better to just leave in the
save-excursion than to make the code more complex.  Thanks for the
clarification.

-- 
This line is not blank.

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> But kill-line is very much a user level function with plenty of
> special features.  Use of kill-line would make `kill-whole-line'
> vulnerable to any change or addition to the user level features of
> kill-line.  I prefer using the more primitive kill-region.

While I liked the simplicity of the code for kill-whole-line when it
used kill-line, it's also clear that many of the checks that
kill-line does are not necessary.  So now I like Luc's version better.
-- 
This line is not blank.

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> The following version of kill-whole-line should work OK with the
> kill-ring, including read-only buffers, and with invisible text.

Nice.  The invisible text becomes visible again.  But that's the same
with C-k.  I presume it's intended to behave like this.

When hitting C-u -2 S-<backspace>, do people expect to kill the current
line, plus the previous line?  Or do people expect to delete the
previous two lines, excluding the current line?

The first would be similar to C-e C-u -1 C-k.
The second would be similar to C-a C-u -2 C-k.

I tend towards the first behavior.

It's interesting that C-k with a negative arg kills more than a C-k
with a positive arg.  If I'm in the middle of a line and do C-u 1 C-k,
then I'll kill half a line.  But if I do C-u -1 C-k, then I'll kill
one-and-a-half lines!  This is quite unexpected.

C-k is similar to C-@ C-e C-w.
C-u 1 C-k is similar to C-@ C-e C-f C-w.

C-u 0 C-k is similar to C-@ C-a C-w.
By analogy, I'd expect C-u -1 C-k to be similar to C-@ C-a C-b C-w.
But it is like C-@ C-a C-p C-w.

What do people think?
-- 
This line is not blank.

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohann wrote:

   Nice.  The invisible text becomes visible again.  But that's the same
   with C-k.  I presume it's intended to behave like this.

Try removing invisible from yank-excluded-properties if you want it
the other way.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohann wrote:

   Okay.  I'm sure someone else can/will commit it.

(Meant is the new version of `kill-whole-line'.)

There in no rush, since there is still some discussion going on.
Maybe I should add some comments to the code, for instance explaining
why the save-excursions are necessary.

Sincerely,

Luc.

Re: Various simple.el patches

[Eli Zaretskii]

> Date: Fri, 23 May 2003 13:27:55 -0500 (CDT)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> I currently have no write access to the Emacs CVS.

That's easily fixed, if you want.

> I may or may not
> want such access depending on exactly how much CVS knowledge is
> necessary to commit changes without fear of doing some horrible
> stupidities.  (I currently know no more CVS than necessary to check
> out the CVS from Savannah.)

You need to know only one additional command: "cvs ci", the one that
commits changes on your local machine to the repository.

Re: Various simple.el patches

[Richard Stallman]

    C-k is similar to C-@ C-e C-w.
    C-u 1 C-k is similar to C-@ C-e C-f C-w.

    C-u 0 C-k is similar to C-@ C-a C-w.
    By analogy, I'd expect C-u -1 C-k to be similar to C-@ C-a C-b C-w.
    But it is like C-@ C-a C-p C-w.

    What do people think?

I think this should not be changed.

Re: Various simple.el patches

[Luc Teirlinck]

You mean that, after making the changes in my own local sources all I
would have to do is:

cvs -d :pserver:to-be-assigned-login-name?:/cvsroot/emacs login
to-be-assigned-password
cvs ci -m '(fictitious-function): new function' emacs/lisp/simple.el

(Where I am assuming that the filename I would have to specify would be the
one in the repository.  My local one would be 
/home/teirllm/CVSemacsdir/emacs/lisp/simple.el)

Sincerely,

Luc.

Re: Various simple.el patches

[Eli Zaretskii]

> Date: Sat, 24 May 2003 21:42:19 -0500 (CDT)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> You mean that, after making the changes in my own local sources all I
> would have to do is:
> 
> cvs -d :pserver:to-be-assigned-login-name?:/cvsroot/emacs login
> to-be-assigned-password
> cvs ci -m '(fictitious-function): new function' emacs/lisp/simple.el

Yes.  Except that "cvs login" is something you do only once on a given
machine.

Re: Various simple.el patches

[Luc Teirlinck]

If things are that straightforward, I actually might be interested in
getting write access to the Emacs CVS.  Is there some danger to
inadvertently leave locks around or similar stuff?  I sometimes have
heard complaints like that on emacs-devel.

Sincerely,

Luc.

Re: Various simple.el patches

[Luc Teirlinck]

>From my previous message:

   Is there some danger to inadvertently leave locks around or similar
   stuff?  I sometimes have heard complaints like that on emacs-devel.

I guess that must have been from doing other things.  It is hard to
see that simple one-line command doing that.

Sincerely,

Luc.

Re: Various simple.el patches

[Stefan Monnier]

> You mean that, after making the changes in my own local sources all I
> would have to do is:
> 
> cvs -d :pserver:to-be-assigned-login-name?:/cvsroot/emacs login
> to-be-assigned-password
> cvs ci -m '(fictitious-function): new function' emacs/lisp/simple.el
> 
> (Where I am assuming that the filename I would have to specify would be the
> one in the repository.  My local one would be 
> /home/teirllm/CVSemacsdir/emacs/lisp/simple.el)

I'd recommend you use a font-end such as VC and/or PCL-CVS
for all of that.


	Stefan

Re: Various simple.el patches

[Stefan Monnier]

> > Date: Sat, 24 May 2003 21:42:19 -0500 (CDT)
> > From: Luc Teirlinck <teirllm@dms.auburn.edu>
> > 
> > You mean that, after making the changes in my own local sources all I
> > would have to do is:
> > 
> > cvs -d :pserver:to-be-assigned-login-name?:/cvsroot/emacs login
> > to-be-assigned-password
> > cvs ci -m '(fictitious-function): new function' emacs/lisp/simple.el
> 
> Yes.  Except that "cvs login" is something you do only once on a given
> machine.

Well, actually, since `login' only applies to pserver access and that
I'm not sure pserver is still available (other than for read-only
anonymous access), there probably won't even be any `login'.
Instead he'll have to setup his ssh to properly authenticate to
subversion.gnu.org.


	Stefan

Re: Various simple.el patches

[Eli Zaretskii]

> Date: Sat, 24 May 2003 23:43:11 -0500 (CDT)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> If things are that straightforward, I actually might be interested in
> getting write access to the Emacs CVS.

Set up an account on savannah and send me your user id.  Then I'll
give you write access.

> Is there some danger to inadvertently leave locks around or similar
> stuff?

No.

> I sometimes have heard complaints like that on emacs-devel.

It never happened to me, so I don't know how did that happen to
others.

Re: Various simple.el patches

[Richard Stallman]

       Is there some danger to inadvertently leave locks around or similar
       stuff?  I sometimes have heard complaints like that on emacs-devel.

    I guess that must have been from doing other things.  It is hard to
    see that simple one-line command doing that.

If you kill the cvs job while it is doing a commit, it leaves the
directory locked.  Or at least, that was true in the past.
Now I think these locks get unlocked automatically after some period
of time elapses, so the problem is not really severe any more.

Re: Various simple.el patches

[Kevin Rodgers]

Richard Stallman wrote:

>     C-k is similar to C-@ C-e C-w.
>     C-u 1 C-k is similar to C-@ C-e C-f C-w.
> 
>     C-u 0 C-k is similar to C-@ C-a C-w.
>     By analogy, I'd expect C-u -1 C-k to be similar to C-@ C-a C-b C-w.
>     But it is like C-@ C-a C-p C-w.
> 
>     What do people think?
> 
> I think this should not be changed.

Why not?  I think Kai's suggestion makes complete sense.


-- 
<a href="mailto:&lt;kevin.rodgers&#64;ihs.com&gt;">Kevin Rodgers</a>

Re: Various simple.el patches

[Luc Teirlinck]

I saw the following entry in the Change Log:

003-05-23  Kai Grossjohann  <kai.grossjohann@gmx.net>

	   * simple.el (kill-region): If nothing was killed, and the
	   previous command was not a kill, break kill sequence.
	   (kill-whole-line): Don't use kill-line.  Make it work with
	   invisible lines.  Make it repeatable.
	   From Luc Teirlinck <teirllm@dms.auburn.edu>.

But it apparently has not been committed (neither the kill-region part
nor the kill-whole-line part.)

I certainly see no problem with the kill-region part, assuming it is
what you proposed in a message to this site.  It seems to implement
exactly what Richard decided, if I understood correctly.

In as far as the kill-whole-line part is concerned, there is something
confusing about the change log entry.  Indeed, the main purpose of the
change was to make kill-whole-line interact correctly with the
kill-ring, both in read-write and read-only buffers.  There were
originally indeed problems with invisible text, but those turned out
*not* to be bugs in the kill-whole-line code itself, but bugs in
forward-visible-line, which I fixed.  In the presence of invisibility
properties that are lists of two or more elements, bugs in
line-move-invisible remain, which still could affect kill-whole-line
as well as other functions.  If I understood correctly, it was decided
that the best way to fix those problems was to make line-move-invisible
a primitive and use existing C code in doing so.  I do not know
whether somebody is actively working on that, or planning to work on
that, at present.

Since 05-23, I made no changes in the actual code of kill-whole-line,
but I added some comments and I propose to change the documentation
string to:

  "Kill current line.
With prefix arg, kill that many lines starting from the current line.
If arg is negative, kill backward.  Also kill the preceding newline.
\(This is meant to make C-x z work well with negative arguments.\)
If arg is zero, kill current line but exclude the trailing newline."

Apart from that, I believe that the Change Log text needs to be
changed for reasons outlined above.

I plan no further changes.

I did not have write access before, but I do now.

What is the most convenient way to proceed from here:

1.  You commit the original change (with maybe a changed Change Log)
    and then I commit the changes I made since.

2.  I send you my latest changes and you commit everything in one piece.

3.  You commit kill-region and I commit all revisions to
    kill-whole-line in one piece.

Sincerely,

Luc.

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> I did not have write access before, but I do now.
> What is the most convenient way to proceed from here:

First of all, let me say I'm sorry that I goofed up.

I'll do whatever you want me to do.  If you want me to commit
something, I'll do that.  If you want me not to commit it, I'll do
that, as well.

One possibility is that I remove the bogus ChangeLog entry and that
you commit your changes with a proper ChangeLog entry.  That way,
credits are where they belong.
-- 
This line is not blank.

Re: Various simple.el patches

[Luc Teirlinck]

Kai Grossjohann wrote:

   One possibility is that I remove the bogus ChangeLog entry and that
   you commit your changes with a proper ChangeLog entry.  That way,
   credits are where they belong.

That might be good.  That way I can see how committing works.  I might
wait until this evening (US central time), because I have other things
to do right now.  Unless there is some reason not to do so, I believe
that you should still commit your change to `kill-region' however,
because it seems to be exactly what we decided.

Sincerely,

Luc.

Re: Various simple.el patches

[Kai Großjohann]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> That might be good.  That way I can see how committing works.  I might
> wait until this evening (US central time), because I have other things
> to do right now.  Unless there is some reason not to do so, I believe
> that you should still commit your change to `kill-region' however,
> because it seems to be exactly what we decided.

Very good!  Done.  (I hope I did it right this time...)
-- 
This line is not blank.

Re: Various simple.el patches

[Richard Stallman]

    Why not?  I think Kai's suggestion makes complete sense.

C-k has worked this way for 18 years, if not 28 years.
It is convenient enough.  Please don't change it.

Re: Various simple.el patches

[Richard Stallman]

    One possibility is that I remove the bogus ChangeLog entry and that
    you commit your changes with a proper ChangeLog entry.  That way,
    credits are where they belong.

Any of us can edit the ChangeLog file, and it does not matter who does
it.  How about if you simply make whatever corrections are needed?