* [PATCH] fix goto-line
@ 2011-07-28 12:57 Jose E. Marchesi
2011-07-28 14:07 ` Juanma Barranquero
0 siblings, 1 reply; 12+ messages in thread
From: Jose E. Marchesi @ 2011-07-28 12:57 UTC (permalink / raw
To: emacs-devel
Hi hackers!
Just a tiny patch fixing a problem in goto-line, which was passing a
string as the second parameter to read-number, triggering an error.
2011-07-28 Jose E. Marchesi <jemarch@gnu.org>
* simple.el (goto-line): Use string-to-number to provide a
numeric argument to read-number.
--- lisp/simple.el 2011-07-16 18:42:38 +0000
+++ lisp/simple.el 2011-07-28 12:47:34 +0000
@@ -900,10 +900,11 @@
(save-excursion
(skip-chars-backward "0-9")
(if (looking-at "[0-9]")
- (buffer-substring-no-properties
+ (string-to-number
+ (buffer-substring-no-properties
(point)
(progn (skip-chars-forward "0-9")
- (point))))))
+ (point)))))))
;; Decide if we're switching buffers.
(buffer
(if (consp current-prefix-arg)
--
Jose E. Marchesi jemarch@gnu.org
GNU Project http://www.gnu.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fix goto-line
2011-07-28 12:57 [PATCH] fix goto-line Jose E. Marchesi
@ 2011-07-28 14:07 ` Juanma Barranquero
2011-07-28 17:38 ` Eduard Wiebe
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Juanma Barranquero @ 2011-07-28 14:07 UTC (permalink / raw
To: Jose E. Marchesi; +Cc: emacs-devel
On Thu, Jul 28, 2011 at 14:57, Jose E. Marchesi <jemarch@gnu.org> wrote:
> Just a tiny patch fixing a problem in goto-line, which was passing a
> string as the second parameter to read-number, triggering an error.
Committed.
Next time please file it as a bug report.
Thanks,
Juanma
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fix goto-line
2011-07-28 14:07 ` Juanma Barranquero
@ 2011-07-28 17:38 ` Eduard Wiebe
2011-07-28 17:38 ` bug#9163: " Eduard Wiebe
2011-07-29 11:15 ` Juri Linkov
2 siblings, 0 replies; 12+ messages in thread
From: Eduard Wiebe @ 2011-07-28 17:38 UTC (permalink / raw
To: emacs-devel; +Cc: 9163
Juanma Barranquero <lekktu@gmail.com> writes:
> On Thu, Jul 28, 2011 at 14:57, Jose E. Marchesi <jemarch@gnu.org> wrote:
>
>> Just a tiny patch fixing a problem in goto-line, which was passing a
>> string as the second parameter to read-number, triggering an error.
>
> Committed.
>
> Next time please file it as a bug report.
Please close bug#9163 ;-)
--
Eduard Wiebe
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9163: [PATCH] fix goto-line
2011-07-28 14:07 ` Juanma Barranquero
2011-07-28 17:38 ` Eduard Wiebe
@ 2011-07-28 17:38 ` Eduard Wiebe
2011-07-29 11:15 ` Juri Linkov
2 siblings, 0 replies; 12+ messages in thread
From: Eduard Wiebe @ 2011-07-28 17:38 UTC (permalink / raw
Cc: 9163
The following message is a courtesy copy of an article
that has been posted to gmane.emacs.devel as well.
Juanma Barranquero <lekktu@gmail.com> writes:
> On Thu, Jul 28, 2011 at 14:57, Jose E. Marchesi <jemarch@gnu.org> wrote:
>
>> Just a tiny patch fixing a problem in goto-line, which was passing a
>> string as the second parameter to read-number, triggering an error.
>
> Committed.
>
> Next time please file it as a bug report.
Please close bug#9163 ;-)
--
Eduard Wiebe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fix goto-line
2011-07-28 14:07 ` Juanma Barranquero
2011-07-28 17:38 ` Eduard Wiebe
2011-07-28 17:38 ` bug#9163: " Eduard Wiebe
@ 2011-07-29 11:15 ` Juri Linkov
2011-07-29 11:22 ` Juanma Barranquero
2 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2011-07-29 11:15 UTC (permalink / raw
To: emacs-devel
>> Just a tiny patch fixing a problem in goto-line, which was passing a
>> string as the second parameter to read-number, triggering an error.
>
> Committed.
I noticed the following areas for improvement:
1. `read-number' already inserts the default value into prompt.
So the same code is not necessary in `goto-line'.
2. It would be useful to add the current line number to the defaults
of `goto-line' to allow its easier modification by users.
This is implemented with:
=== modified file 'lisp/simple.el'
--- lisp/simple.el 2011-07-28 14:05:07 +0000
+++ lisp/simple.el 2011-07-29 11:14:04 +0000
@@ -914,11 +914,8 @@ (defun goto-line (line &optional buffer)
(concat " in " (buffer-name buffer))
"")))
;; Read the argument, offering that number (if any) as default.
- (list (read-number (format (if default "Goto line%s (%s): "
- "Goto line%s: ")
- buffer-prompt
- default)
- default)
+ (list (read-number (format "Goto line%s: " buffer-prompt)
+ (list default (line-number-at-pos)))
buffer))))
;; Switch to the desired buffer, one way or another.
(if buffer
3. `read-number' still doesn't support multiple default values
like other minibuffer reading functions. Let's do it.
=== modified file 'lisp/subr.el'
--- lisp/subr.el 2011-07-15 23:59:25 +0000
+++ lisp/subr.el 2011-07-29 11:12:45 +0000
@@ -2113,23 +2113,27 @@ (defun read-number (prompt &optional def
"Read a numeric value in the minibuffer, prompting with PROMPT.
DEFAULT specifies a default value to return if the user just types RET.
The value of DEFAULT is inserted into PROMPT."
- (let ((n nil))
- (when default
+ (let ((n nil)
+ (default1 (if (consp default) (car default) default)))
+ (when default1
(setq prompt
(if (string-match "\\(\\):[ \t]*\\'" prompt)
- (replace-match (format " (default %s)" default) t t prompt 1)
+ (replace-match (format " (default %s)" default1) t t prompt 1)
(replace-regexp-in-string "[ \t]*\\'"
- (format " (default %s) " default)
+ (format " (default %s) " default1)
prompt t t))))
(while
(progn
- (let ((str (read-from-minibuffer prompt nil nil nil nil
- (and default
- (number-to-string default)))))
+ (let ((str (read-from-minibuffer
+ prompt nil nil nil nil
+ (when default
+ (if (consp default)
+ (mapcar 'number-to-string (delq nil default))
+ (number-to-string default))))))
(condition-case nil
(setq n (cond
- ((zerop (length str)) default)
- ((stringp str) (read str))))
+ ((zerop (length str)) default1)
+ ((stringp str) (string-to-number str))))
(error nil)))
(unless (numberp n)
(message "Please enter a number.")
4. In the patch above I replaced `read' with `string-to-number'
for consistency.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fix goto-line
2011-07-29 11:15 ` Juri Linkov
@ 2011-07-29 11:22 ` Juanma Barranquero
2011-07-29 15:28 ` Juri Linkov
0 siblings, 1 reply; 12+ messages in thread
From: Juanma Barranquero @ 2011-07-29 11:22 UTC (permalink / raw
To: Juri Linkov; +Cc: emacs-devel
On Fri, Jul 29, 2011 at 13:15, Juri Linkov <juri@jurta.org> wrote:
> I noticed the following areas for improvement:
>
> 1. `read-number' already inserts the default value into prompt.
> So the same code is not necessary in `goto-line'.
>
> 2. It would be useful to add the current line number to the defaults
> of `goto-line' to allow its easier modification by users.
There are improvements and not bug fixes, so please file this as a
wishlist item for 24.2 (and do not forget to add the "patch" tag).
Juanma
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fix goto-line
2011-07-29 11:22 ` Juanma Barranquero
@ 2011-07-29 15:28 ` Juri Linkov
2011-07-29 16:45 ` Paul Eggert
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2011-07-29 15:28 UTC (permalink / raw
To: Juanma Barranquero; +Cc: emacs-devel
> There are improvements and not bug fixes, so please file this as a
> wishlist item for 24.2 (and do not forget to add the "patch" tag).
These are improvements, but changes in the default value of `dired-do-touch'
is a regression. I'd like to fix this regression with the following patch.
It restores the old pre-24 essential behavior, but still keeps the
requirement asked by the reporter of bug#6887 to use the current time.
More changes from http://thread.gmane.org/gmane.emacs.devel/81414/focus=82988
could be postponed to 24.2.
=== modified file 'lisp/dired-aux.el'
--- lisp/dired-aux.el 2011-07-25 08:23:29 +0000
+++ lisp/dired-aux.el 2011-07-29 15:27:45 +0000
@@ -227,20 +227,6 @@ (defun dired-files-attributes (dir)
(directory-files dir)))
\f
-(defun dired-touch-initial (files)
- "Create initial input value for `touch' command."
- ;; Nobody can explain what this version is supposed to do. (Bug#6887)
- ;; Also, the manual says it uses "the present time".
- ;;; (let (initial)
- ;;; (while files
- ;;; (let ((current (nth 5 (file-attributes (car files)))))
- ;;; (if (and initial (not (equal initial current)))
- ;;; (setq initial (current-time) files nil)
- ;;; (setq initial current))
- ;;; (setq files (cdr files))))
- ;;; (format-time-string "%Y%m%d%H%M.%S" initial)))
- (format-time-string "%Y%m%d%H%M.%S" (current-time)))
-
(defun dired-do-chxxx (attribute-name program op-symbol arg)
;; Change file attributes (mode, group, owner, timestamp) of marked files and
;; refresh their file lines.
@@ -249,11 +235,18 @@ (defun dired-do-chxxx (attribute-name pr
;; OP-SYMBOL is the type of operation (for use in dired-mark-pop-up).
;; ARG describes which files to use, as in dired-get-marked-files.
(let* ((files (dired-get-marked-files t arg))
+ (initial
+ (if (eq op-symbol 'touch)
+ (format-time-string "%Y%m%d%H%M.%S" (current-time))))
+ (default
+ (if (eq op-symbol 'touch)
+ (and (stringp (car files))
+ (format-time-string "%Y%m%d%H%M.%S"
+ (nth 5 (file-attributes (car files)))))))
(new-attribute
(dired-mark-read-string
(concat "Change " attribute-name " of %s to: ")
- (if (eq op-symbol 'touch) (dired-touch-initial files))
- op-symbol arg files))
+ initial op-symbol arg files default))
(operation (concat program " " new-attribute))
failures)
(setq failures
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] fix goto-line
2011-07-29 15:28 ` Juri Linkov
@ 2011-07-29 16:45 ` Paul Eggert
2011-07-30 9:17 ` dired-do-touch (was: [PATCH] fix goto-line) Juri Linkov
0 siblings, 1 reply; 12+ messages in thread
From: Paul Eggert @ 2011-07-29 16:45 UTC (permalink / raw
To: Juri Linkov; +Cc: Juanma Barranquero, emacs-devel
On 07/29/11 08:28, Juri Linkov wrote:
> I'd like to fix this regression with the following patch.
Thanks, could you please open up a bug report for this
on debbugs.gnu.org? That'd be a better place to discuss
this anyway.
> + (if (eq op-symbol 'touch)
> + (format-time-string "%Y%m%d%H%M.%S" (current-time))))
I don't know what the overall problem is here, but I can
see two issues with this proposed patch. First, presumably it
invokes (current-time) at one point, and then, later,
uses that saved current-time to do the 'touch'. But that's
not quite right; it's the equivalent of the shell command
touch --date="$(date)" file
whereas surely what is wanted is the equivalent of
touch file
The latter command avoids some race conditions, because it
uses the time at the point of the 'touch', not at the point of
the invocation of 'date' (or of 'current-time').
The second issue is minor, and perhaps fixing the first issue
will make it irrelevant, but here it is anyway: the
"(current-time)" can be omitted in the above code, as it's the default
for that argument of format-time-string.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dired-do-touch (was: [PATCH] fix goto-line)
2011-07-29 16:45 ` Paul Eggert
@ 2011-07-30 9:17 ` Juri Linkov
2011-07-30 9:50 ` dired-do-touch Juri Linkov
2011-07-30 9:54 ` dired-do-touch (was: [PATCH] fix goto-line) Andreas Schwab
0 siblings, 2 replies; 12+ messages in thread
From: Juri Linkov @ 2011-07-30 9:17 UTC (permalink / raw
To: Paul Eggert; +Cc: emacs-devel
> Thanks, could you please open up a bug report for this
> on debbugs.gnu.org? That'd be a better place to discuss
> this anyway.
I've unarchived that bug report and referred to this thread from it.
>> + (if (eq op-symbol 'touch)
>> + (format-time-string "%Y%m%d%H%M.%S" (current-time))))
>
> I don't know what the overall problem is here, but I can
> see two issues with this proposed patch. First, presumably it
> invokes (current-time) at one point, and then, later,
> uses that saved current-time to do the 'touch'. But that's
> not quite right; it's the equivalent of the shell command
>
> touch --date="$(date)" file
In theory you are right. But in practice a 1-2 sec delay
is not a problem.
> whereas surely what is wanted is the equivalent of
>
> touch file
When `touch file' is really wanted then it's easy to type `! touch'.
`dired-do-touch' was created for the convenience of changing
the file's timestamp providing useful default values for editing.
> The latter command avoids some race conditions, because it
> uses the time at the point of the 'touch', not at the point of
> the invocation of 'date' (or of 'current-time').
`T' is intended to allow editing the file attribute of the last
modification time, like `M' is used for editing the file's mode,
`O' - owner's UID, `G' - GID. The docstring of `dired-do-touch' says:
"Change the timestamp of the marked (or next ARG) files."
And Dired is "Directory Editor" after all, so it should allow
editing file attributes including timestamps.
> The second issue is minor, and perhaps fixing the first issue
> will make it irrelevant, but here it is anyway: the
> "(current-time)" can be omitted in the above code, as it's the default
> for that argument of format-time-string.
Thanks, I removed "(current-time)" from the call to `format-time-string'.
BTW, I noticed that the docstring of `format-time-string' refers
to the argument `TIME' whereas the real argument name is `TIMEVAL'.
Adding something like "\n\(fn FORMAT-STRING &optional TIME UNIVERSAL)"
to the last line of the docstring would help.
Another documentation mismatch is that (info "(emacs) Operating on Files")
says:
`M MODESPEC <RET>'
Change the mode (also called "permission bits") of the specified
files (`dired-do-chmod'). This uses the `chmod' program, so
MODESPEC can be any argument that `chmod' can handle.
But actually `dired-do-chmod' doesn't use the `chmod' program anymore.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dired-do-touch
2011-07-30 9:17 ` dired-do-touch (was: [PATCH] fix goto-line) Juri Linkov
@ 2011-07-30 9:50 ` Juri Linkov
2011-07-30 9:54 ` dired-do-touch (was: [PATCH] fix goto-line) Andreas Schwab
1 sibling, 0 replies; 12+ messages in thread
From: Juri Linkov @ 2011-07-30 9:50 UTC (permalink / raw
To: Paul Eggert; +Cc: emacs-devel
> BTW, I noticed that the docstring of `format-time-string' refers
> to the argument `TIME' whereas the real argument name is `TIMEVAL'.
> Adding something like "\n\(fn FORMAT-STRING &optional TIME UNIVERSAL)"
> to the last line of the docstring would help.
The "\(fn" syntax is used only for the docstrings of Lisp functions.
So it should be
"usage: (format-time-string FORMAT-STRING &optional TIME UNIVERSAL)"
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dired-do-touch (was: [PATCH] fix goto-line)
2011-07-30 9:17 ` dired-do-touch (was: [PATCH] fix goto-line) Juri Linkov
2011-07-30 9:50 ` dired-do-touch Juri Linkov
@ 2011-07-30 9:54 ` Andreas Schwab
2011-07-30 11:01 ` dired-do-touch Juri Linkov
1 sibling, 1 reply; 12+ messages in thread
From: Andreas Schwab @ 2011-07-30 9:54 UTC (permalink / raw
To: Juri Linkov; +Cc: Paul Eggert, emacs-devel
Juri Linkov <juri@jurta.org> writes:
>>> + (if (eq op-symbol 'touch)
>>> + (format-time-string "%Y%m%d%H%M.%S" (current-time))))
>>
>> I don't know what the overall problem is here, but I can
>> see two issues with this proposed patch. First, presumably it
>> invokes (current-time) at one point, and then, later,
>> uses that saved current-time to do the 'touch'. But that's
>> not quite right; it's the equivalent of the shell command
>>
>> touch --date="$(date)" file
>
> In theory you are right. But in practice a 1-2 sec delay
> is not a problem.
Note that changing the timestamp to a specific value requires owner
rights, whereas setting it to the current time only requires write
access.
>> whereas surely what is wanted is the equivalent of
>>
>> touch file
>
> When `touch file' is really wanted then it's easy to type `! touch'.
> `dired-do-touch' was created for the convenience of changing
> the file's timestamp providing useful default values for editing.
An empty timestamp should mean running touch without time argument.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-07-30 11:01 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-28 12:57 [PATCH] fix goto-line Jose E. Marchesi
2011-07-28 14:07 ` Juanma Barranquero
2011-07-28 17:38 ` Eduard Wiebe
2011-07-28 17:38 ` bug#9163: " Eduard Wiebe
2011-07-29 11:15 ` Juri Linkov
2011-07-29 11:22 ` Juanma Barranquero
2011-07-29 15:28 ` Juri Linkov
2011-07-29 16:45 ` Paul Eggert
2011-07-30 9:17 ` dired-do-touch (was: [PATCH] fix goto-line) Juri Linkov
2011-07-30 9:50 ` dired-do-touch Juri Linkov
2011-07-30 9:54 ` dired-do-touch (was: [PATCH] fix goto-line) Andreas Schwab
2011-07-30 11:01 ` dired-do-touch Juri Linkov
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.