* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
@ 2014-06-20 8:09 Dima Kogan
2014-06-20 8:32 ` Michael Albinus
0 siblings, 1 reply; 7+ messages in thread
From: Dima Kogan @ 2014-06-20 8:09 UTC (permalink / raw
To: 17815
Hi.
The documentation is a bit unclear, so I'm not 100% sure this is a bug;
it's definitely a surprising behavior, though.
I have a bit of elisp to create a temporary buffer and to run a process,
sending its output to this buffer:
(let ((output-buffer (get-buffer-create "*test-buf*")))
(with-current-buffer output-buffer
(erase-buffer)
(let ((default-directory "/tmp"))
(process-file "whoami" nil output-buffer t))))
Note that I do not ask for this buffer to be raised. On my machine
(Debian/sid amd64) this indeed does not raise the *test-buf* buffer, and
I do not even see it if I don't explicitly switch to it. This is good.
If I change the directory from "/tmp" to any TRAMP path (for instance
"/sudo::/tmp") then this elisp DOES raise *test-buf*. This difference
between normal and TRAMP behavior sounds like a bug to me.
Note that I have (process-file ... ... ... t). Changing this to nil
resolves the issue. The documentation says
Fourth arg DISPLAY non-nil means redisplay buffer as output is
inserted.
I don't know if "redisplay" includes "raise", but I do think the
behavior should be the same, TRAMP or not.
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
2014-06-20 8:09 bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP Dima Kogan
@ 2014-06-20 8:32 ` Michael Albinus
2014-06-20 13:09 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Michael Albinus @ 2014-06-20 8:32 UTC (permalink / raw
To: Dima Kogan; +Cc: 17815
Dima Kogan <dima@secretsauce.net> writes:
> Hi.
Hi Dima,
> If I change the directory from "/tmp" to any TRAMP path (for instance
> "/sudo::/tmp") then this elisp DOES raise *test-buf*. This difference
> between normal and TRAMP behavior sounds like a bug to me.
I could reproduce it locally. And I agree, it is rather a bug in Tramp,
which I will fix next days (being too busy just now).
@Stefan: This is no regression, I could reproduce it even with Emacs
23.4. Therefore, I will fix it in the trunk. Please tell me if you
believe it shall go into emacs-24.
> Thanks
Thanks for reporting, and best regards, Michael.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
2014-06-20 8:32 ` Michael Albinus
@ 2014-06-20 13:09 ` Stefan Monnier
2014-06-20 13:50 ` Michael Albinus
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2014-06-20 13:09 UTC (permalink / raw
To: Michael Albinus; +Cc: Dima Kogan, 17815
> @Stefan: This is no regression, I could reproduce it even with Emacs
> 23.4. Therefore, I will fix it in the trunk. Please tell me if you
> believe it shall go into emacs-24.
Show me the patch (when it's ready), so I can see whether it looks
safe enough.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
2014-06-20 13:09 ` Stefan Monnier
@ 2014-06-20 13:50 ` Michael Albinus
[not found] ` <jwvwqcbprk7.fsf-monnier+emacsbugs@gnu.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michael Albinus @ 2014-06-20 13:50 UTC (permalink / raw
To: Stefan Monnier; +Cc: Dima Kogan, 17815
[-- Attachment #1: Type: text/plain, Size: 371 bytes --]
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> @Stefan: This is no regression, I could reproduce it even with Emacs
>> 23.4. Therefore, I will fix it in the trunk. Please tell me if you
>> believe it shall go into emacs-24.
>
> Show me the patch (when it's ready), so I can see whether it looks
> safe enough.
That's what I've committed to the Tramp repository:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: diff --]
[-- Type: text/x-patch, Size: 5121 bytes --]
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 76a3c48..ba410f1 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,10 @@
+2014-06-20 Michael Albinus <michael.albinus@gmx.de>
+
+ * tramp-adb.el (tramp-adb-handle-process-file):
+ * tramp-sh.el (tramp-sh-handle-process-file):
+ * tramp-smb.el (tramp-smb-handle-process-file): Do not raise the
+ output buffer when DISPLAY is non-nil. (Bug#17815)
+
2014-06-16 Michael Albinus <michael.albinus@gmx.de>
* tramp.el (tramp-call-process): Handle error strings.
diff --git a/lisp/tramp-adb.el b/lisp/tramp-adb.el
index f38cecb..91caa4a 100644
--- a/lisp/tramp-adb.el
+++ b/lisp/tramp-adb.el
@@ -801,11 +801,11 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored."
v (format "(cd %s; %s)"
(tramp-shell-quote-argument localname) command)
"")
- ;; We should show the output anyway.
+ ;; We should add the output anyway.
(when outbuf
(with-current-buffer outbuf
(insert-buffer-substring (tramp-get-connection-buffer v)))
- (when display (display-buffer outbuf))))
+ (when (and display (get-buffer-window outbuf t)) (redisplay))))
;; When the user did interrupt, we should do it also. We use
;; return code -1 as marker.
(quit
diff --git a/lisp/tramp-sh.el b/lisp/tramp-sh.el
index a6771cd..68f1ef4 100644
--- a/lisp/tramp-sh.el
+++ b/lisp/tramp-sh.el
@@ -2994,13 +2994,13 @@ the result will be a local, non-Tramp, file name."
command)
t t)
0 1))
- ;; We should show the output anyway.
+ ;; We should add the output anyway.
(when outbuf
(with-current-buffer outbuf
(insert
(with-current-buffer (tramp-get-connection-buffer v)
(buffer-string))))
- (when display (display-buffer outbuf))))
+ (when (and display (get-buffer-window outbuf t)) (redisplay))))
;; When the user did interrupt, we should do it also. We use
;; return code -1 as marker.
(quit
diff --git a/lisp/tramp-smb.el b/lisp/tramp-smb.el
index aa44b8d..15ae9ed 100644
--- a/lisp/tramp-smb.el
+++ b/lisp/tramp-smb.el
@@ -1225,8 +1225,8 @@ target of the symlink differ."
(error
(setq ret 1)))
- ;; We should show the output anyway.
- (when (and outbuf display) (display-buffer outbuf))
+ ;; We should redisplay the output.
+ (when (and display outbuf (get-buffer-window outbuf t)) (redisplay))
;; Cleanup. We remove all file cache values for the connection,
;; because the remote process could have changed them.
diff --git a/test/ChangeLog b/test/ChangeLog
index c672532..5ba0b82 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,8 @@
+2014-06-20 Michael Albinus <michael.albinus@gmx.de>
+
+ * tramp-tests.el (tramp-test26-process-file): Extend test
+ according to Bug#17815.
+
2014-06-15 Michael Albinus <michael.albinus@gmx.de>
Version 2.2.10 released.
diff --git a/test/tramp-tests.el b/test/tramp-tests.el
index d30a5b0..b010ab4 100644
--- a/test/tramp-tests.el
+++ b/test/tramp-tests.el
@@ -1246,9 +1246,10 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
(tramp-find-foreign-file-name-handler tramp-test-temporary-file-directory)
'(tramp-gvfs-file-name-handler tramp-smb-file-name-handler))))
- (let ((tmp-name (tramp--test-make-temp-name))
- (default-directory tramp-test-temporary-file-directory)
- kill-buffer-query-functions)
+ (let* ((tmp-name (tramp--test-make-temp-name))
+ (fnnd (file-name-nondirectory tmp-name))
+ (default-directory tramp-test-temporary-file-directory)
+ kill-buffer-query-functions)
(unwind-protect
(progn
;; We cannot use "/bin/true" and "/bin/false"; those paths
@@ -1259,17 +1260,25 @@ This tests also `make-symbolic-link', `file-truename' and `add-name-to-file'."
(with-temp-buffer
(write-region "foo" nil tmp-name)
(should (file-exists-p tmp-name))
- (should
- (zerop
- (process-file "ls" nil t nil (file-name-nondirectory tmp-name))))
+ (should (zerop (process-file "ls" nil t nil fnnd)))
+ ;; `ls' could produce colorized output.
+ (goto-char (point-min))
+ (while (re-search-forward tramp-color-escape-sequence-regexp nil t)
+ (replace-match "" nil nil))
+ (should (string-equal (format "%s\n" fnnd) (buffer-string)))
+ (should-not (get-buffer-window (current-buffer) t))
+
+ ;; Second run. The output must be appended.
+ (should (zerop (process-file "ls" nil t t fnnd)))
;; `ls' could produce colorized output.
(goto-char (point-min))
(while (re-search-forward tramp-color-escape-sequence-regexp nil t)
(replace-match "" nil nil))
(should
- (string-equal
- (format "%s\n" (file-name-nondirectory tmp-name))
- (buffer-string)))))
+ (string-equal (format "%s\n%s\n" fnnd fnnd) (buffer-string)))
+ ;; A non-nil DISPLAY must not raise the buffer.
+ (should-not (get-buffer-window (current-buffer) t))))
+
(ignore-errors (delete-file tmp-name)))))
(ert-deftest tramp-test27-start-file-process ()
[-- Attachment #3: Type: text/plain, Size: 42 bytes --]
> Stefan
Best regards, Michael.
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
[not found] ` <jwvwqcbprk7.fsf-monnier+emacsbugs@gnu.org>
@ 2014-06-22 9:28 ` Michael Albinus
2014-06-22 12:55 ` Stefan Monnier
0 siblings, 1 reply; 7+ messages in thread
From: Michael Albinus @ 2014-06-22 9:28 UTC (permalink / raw
To: Stefan Monnier; +Cc: Dima Kogan, 17815-done
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>> @Stefan: This is no regression, I could reproduce it even with Emacs
>>>> 23.4. Therefore, I will fix it in the trunk. Please tell me if you
>>>> believe it shall go into emacs-24.
>>> Show me the patch (when it's ready), so I can see whether it looks
>>> safe enough.
>> That's what I've committed to the Tramp repository:
>
> Looks safe enough for emacs-24, thanks.
I've committed the lisp files to the emacs-24 branch as 117284, closing
the bug. tramp-tests.el will be committed to the trunk, next time
emacs-24 has been merged there.
> And in trunk, could you try and reduce the code-duplication between
> tramp-sh.el and tramp-adb.el?
Well, all handlers I could factor out for several backends, live in
tramp.el as `tramp-handle-...'. `tramp-adb-handle-process-file' and
`tramp-sh-handle-process-file' contain subtle differences, it will be
harder to refactor them.
> Stefan
Best regards, Michael.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
2014-06-22 9:28 ` Michael Albinus
@ 2014-06-22 12:55 ` Stefan Monnier
2014-06-22 13:46 ` Michael Albinus
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2014-06-22 12:55 UTC (permalink / raw
To: Michael Albinus; +Cc: Dima Kogan, 17815-done
> Well, all handlers I could factor out for several backends, live in
> tramp.el as `tramp-handle-...'. `tramp-adb-handle-process-file' and
> `tramp-sh-handle-process-file' contain subtle differences, it will be
> harder to refactor them.
Those functions are almost 100 lines long and yet the diff between the
two is only the little thing below. Clearly, there's room for a good
refactoring. Maybe you can't replace them with a single function, but
you can create a third function that holds most of the code.
Stefan
--- mine/tramp-sh.el
+++ other/tramp-sh.el
@@ -1,4 +1,4 @@
-(defun tramp-sh-handle-process-file
+(defun tramp-adb-handle-process-file
(program &optional infile destination display &rest args)
"Like `process-file' for Tramp files."
;; The implementation is not complete yet.
@@ -66,20 +66,16 @@
;; it. Call it in a subshell, in order to preserve working
;; directory.
(condition-case nil
- (unwind-protect
- (setq ret
- (if (tramp-send-command-and-check
- v (format "\\cd %s; %s"
- (tramp-shell-quote-argument localname)
- command)
- t t)
- 0 1))
+ (progn
+ (setq ret 0)
+ (tramp-adb-barf-unless-okay
+ v (format "(cd %s; %s)"
+ (tramp-shell-quote-argument localname) command)
+ "")
;; We should add the output anyway.
(when outbuf
(with-current-buffer outbuf
- (insert
- (with-current-buffer (tramp-get-connection-buffer v)
- (buffer-string))))
+ (insert-buffer-substring (tramp-get-connection-buffer v)))
(when (and display (get-buffer-window outbuf t)) (redisplay))))
;; When the user did interrupt, we should do it also. We use
;; return code -1 as marker.
@@ -101,7 +97,7 @@
;; `process-file-side-effects' has been introduced with GNU
;; Emacs 23.2. If set to `nil', no remote file will be changed
;; by `program'. If it doesn't exist, we assume its default
- ;; value `t'.
+ ;; value 't'.
(unless (and (boundp 'process-file-side-effects)
(not (symbol-value 'process-file-side-effects)))
(tramp-flush-directory-property v ""))
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP
2014-06-22 12:55 ` Stefan Monnier
@ 2014-06-22 13:46 ` Michael Albinus
0 siblings, 0 replies; 7+ messages in thread
From: Michael Albinus @ 2014-06-22 13:46 UTC (permalink / raw
To: Stefan Monnier; +Cc: Dima Kogan, 17815
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Well, all handlers I could factor out for several backends, live in
>> tramp.el as `tramp-handle-...'. `tramp-adb-handle-process-file' and
>> `tramp-sh-handle-process-file' contain subtle differences, it will be
>> harder to refactor them.
>
> Those functions are almost 100 lines long and yet the diff between the
> two is only the little thing below.
Right, that's why I wrote about "subtle" differences.
> Clearly, there's room for a good refactoring. Maybe you can't replace
> them with a single function, but you can create a third function that
> holds most of the code.
Sure. But lately I was plagued with recursive loading of Tramp packages,
so I'm a little bit conservative in moving code between the different
files.
Whenever it is possible to refactor code out I'll do. *-process-file is
on the list, but I cannot promise to do it immediately. One idea is to
generalize `tramp-send-command' and friends, for most of the handlers
this is the major difference between the different backends. Other
handlers but *-process-file would profit from this as well.
> Stefan
Best regards, Michael.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-06-22 13:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-20 8:09 bug#17815: 24.4.50; (process-file) erroneously raises its buffer when running with TRAMP Dima Kogan
2014-06-20 8:32 ` Michael Albinus
2014-06-20 13:09 ` Stefan Monnier
2014-06-20 13:50 ` Michael Albinus
[not found] ` <jwvwqcbprk7.fsf-monnier+emacsbugs@gnu.org>
2014-06-22 9:28 ` Michael Albinus
2014-06-22 12:55 ` Stefan Monnier
2014-06-22 13:46 ` Michael Albinus
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.