From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.help Subject: Re: tramp-adb.el (Access Android devices filesystem using Emacs) Date: Sun, 15 May 2011 21:55:32 +0200 Message-ID: <871uzzivrf.fsf@gmx.de> References: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: dough.gmane.org 1305489353 32144 80.91.229.12 (15 May 2011 19:55:53 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sun, 15 May 2011 19:55:53 +0000 (UTC) Cc: help-gnu-emacs , tramp-devel@gnu.org To: =?utf-8?Q?J=C3=BCrgen_H=C3=B6tzel?= Original-X-From: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Sun May 15 21:55:49 2011 Return-path: Envelope-to: geh-help-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1QLhPn-000508-Oe for geh-help-gnu-emacs@m.gmane.org; Sun, 15 May 2011 21:55:48 +0200 Original-Received: from localhost ([::1]:58820 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QLhPn-0003V9-Bw for geh-help-gnu-emacs@m.gmane.org; Sun, 15 May 2011 15:55:47 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:35143) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QLhPg-0003Tj-Qb for help-gnu-emacs@gnu.org; Sun, 15 May 2011 15:55:42 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QLhPd-0005Ic-S5 for help-gnu-emacs@gnu.org; Sun, 15 May 2011 15:55:40 -0400 Original-Received: from mailout-de.gmx.net ([213.165.64.22]:53213) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1QLhPd-0005IF-EB for help-gnu-emacs@gnu.org; Sun, 15 May 2011 15:55:37 -0400 Original-Received: (qmail invoked by alias); 15 May 2011 19:55:34 -0000 Original-Received: from p57BB8457.dip0.t-ipconnect.de (EHLO detlef.gmx.de) [87.187.132.87] by mail.gmx.net (mp020) with SMTP; 15 May 2011 21:55:34 +0200 X-Authenticated: #3708877 X-Provags-ID: V01U2FsdGVkX18dfhBPjYsd2gc8CZl50+TEspEzB+Tmje14MqRUnU /R+yyV0b1ws/mX User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-Y-GMX-Trusted: 0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 213.165.64.22 X-BeenThere: help-gnu-emacs@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Users list for the GNU Emacs text editor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Original-Sender: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.help:80984 Archived-At: J=C3=BCrgen H=C3=B6tzel writes: Hi Juergen, > Although The Android shell and file utilities are quite limited > (missing commands like "test" or features like "ls --dired"), tramp- > adb is already quite useful for me: > > https://github.com/juergenhoetzel/tramp-adb > > Any feedback/contribution appreciated Nice package. I don't use adb myself (yet), therefore just some comments by code reading: 25 (require 'tramp) 26=20 27 (defcustom tramp-adb-sdk-dir "~/Android/sdk" 28 "Set to the directory containing the Android SDK." 29 :type 'string 30 :group 'tramp-adb) 31=20 32 (defconst tramp-adb-method "adb" 33 "*When this method name is used, forward all calls to Android Debug B= ridge.") You do not use the ";;;###tramp-autoload" cookie. It would be helpful to load tramp-adb.el without explicitely requiring it in .emacs. 37 (add-to-list 'tramp-methods `(,tramp-adb-method)) 38=20 39 (add-to-list 'tramp-default-method-alist 40 (list "\\`adb" nil tramp-adb-method)) 41=20 42 (add-to-list 'tramp-methods (cons tramp-adb-method nil)) Why do you add tramp-adb-method twice to tramp-methods? 47 (defconst tramp-adb-file-name-handler-alist 48 '((directory-file-name . tramp-handle-directory-file-name) 49 (dired-uncache . tramp-handle-dired-uncache) 50 (file-name-as-directory . tramp-handle-file-name-as-directory) 51 (file-name-completion . tramp-handle-file-name-completion) 52 (file-name-all-completions . tramp-adb-handle-file-name-all-complet= ions) 53 (file-attributes . tramp-adb-handle-file-attributes) 54 (file-name-directory . tramp-handle-file-name-directory) 55 (file-name-nondirectory . tramp-handle-file-name-nondirectory) 56 (file-newer-than-file-p . tramp-handle-file-newer-than-file-p) 57 (file-regular-p . tramp-handle-file-regular-p) 58 (file-remote-p . tramp-handle-file-remote-p) 59 (file-directory-p . tramp-adb-handle-file-directory-p) 60 (file-symlink-p . tramp-handle-file-symlink-p) 61 (file-exists-p . tramp-adb-handle-file-exists-p) 62 (file-writable-p . tramp-adb-handle-file-writable-p) 63 (file-local-copy . tramp-adb-handle-file-local-copy) 64 (expand-file-name . tramp-adb-handle-expand-file-name) 65 (find-backup-file-name . tramp-handle-find-backup-file-name) 66 (directory-files . tramp-adb-handle-directory-files) 67 (make-directory . tramp-adb-handle-make-directory) 68 (delete-directory . tramp-adb-handle-delete-directory) 69 (delete-file . tramp-adb-handle-delete-file) 70 (load . tramp-handle-load) 71 (insert-directory . tramp-adb-handle-insert-directory) 72 (insert-file-contents . tramp-handle-insert-file-contents) 73 (substitute-in-file-name . tramp-handle-substitute-in-file-name) 74 (unhandled-file-name-directory . tramp-handle-unhandled-file-name-d= irectory) 75 (vc-registered . ignore) ;no vc control files on Android devices 76 (write-region . tramp-adb-handle-write-region) 77 (rename-file . tramp-sh-handle-rename-file))=09 78 "Alist of handler functions for Tramp ADB method.") Some functions are missing. You have a handler for rename-file, but none for copy-file. And if you want to reuse tramp-sh-handle-rename-file, you would need to load tramp-sh.el. 93 (with-current-buffer (get-buffer-create "my-buff") 94 (insert (format "op: %s, args: %s\n" operation args))) What's that good for? If you need traces, it might be better to use tramp-message. Or, alternatively, you profile this function with elp. 113 (tramp-message v 5 "tramp-adb-handle-expand-file-name returns: %s" r) You don't need to write the function name in tramp-message's strings. The function name is always added by tramp-message itself. 117 (defun tramp-adb-handle-file-directory-p (filename) 118 "Like `file-directory-p' for Tramp files." 119 (and (file-exists-p filename) 120 (car (file-attributes filename)))) In theory, (car (file-attributes ...)) could also return a string (in case of symlinks). The test would be better to check (eq t (car ...)) 127 (tramp-message v 5 "adb: file attributes with ls: %s" localname) Again, you dont need the "adb" identification in the message. 157 (defun tramp-adb--gnu-switches-to-ash 158 (switches) 159 "Almquist shell can't handle multiple arguments. Convert (\"-al\") to= (\"-a\" \"-l\")" 160 (split-string (apply 'concat (mapcar (lambda (s) 161 (replace-regexp-in-string "\\(.\\)" " -\\1"=20 162 (replace-regexp-in-string "^-" "" s)))=20 163 ;; FIXME: Warning about removed switches (long and non-dash) 164 (remove-if (lambda (s)=20 165 (string-match "\\(^--\\|^[^-]\\)" s)) switches))))) Please restrict yourself to the 80 chars/line limit. The code is readable, then. 208 (tramp-message v 6 "mkdir doesn't handle \"-p\" switch: mkdir \"%= s\"" (tramp-shell-quote-argument localname))) Verbose level 6 shall be restricted to sent commands and the returned output. This allows reading of trace buffers. 209 (save-excursion 210 (tramp-barf-unless-okay 211 v (format "%s %s" 212 "mkdir" 213 (tramp-shell-quote-argument localname)) 214 "Couldn't make directory %s" dir) tramp-barf-unless-okay calls tramp-send-command, you use tramp-adb-send-command. Are both functions equivalent? 221 (tramp-flush-file-property v (file-name-directory localname)) 222 (tramp-flush-directory-property v localname) You have kept the tramp-flush-* functions. It might be a good idea to apply the property setting functions as well. 253 (tramp-adb-handle-directory-files directory))))))) In general, we don't call the tramp-*-handle-* functions directly. This bypasses the mechanisms in tramp-*-file-name-handler functions, which prevent sometimes subtle locking situations. Therefore, we call the primitive functions directly, whenever possible. This case, directory-files could be called. 329 (defun tramp-adb-execute-adb-command (&rest args) 330 "Returns nil on success error-output on failure." 331 (let ((adb-program (concat (file-name-as-directory tramp-adb-sdk-dir)= "platform-tools/adb"))) 332 (with-temp-buffer=20 333 (unless (zerop (apply 'call-process-shell-command adb-program nil= t nil args)) 334 (buffer-string))))) I do not know too much about adb. Is it necessary to call adb again and again? Or does it have an interactive mode (like smbclient), which could be used? The latter case, if possible, would perform better IMHO. 383 (adb-program (concat (file-name-as-directory tramp-adb-sdk-dir) "plat= form-tools/adb"))) expand-file-name could be useful. > J=C3=BCrgen Best regards, Michael.