* 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
[parent not found: <jwvwqcbprk7.fsf-monnier+emacsbugs@gnu.org>]
* 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 public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).