tramp-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Enable adb port in tramp adb


From: Zhongwei Yao
Subject: Re: Enable adb port in tramp adb
Date: Fri, 16 Jan 2015 00:58:09 +0800

Hi, Michael,
Thank so much for your waiting for my legal progress! It takes much longer than I expected.

On Wed, Jan 14, 2015 at 11:36 PM, Michael Albinus <address@hidden> wrote:
Ashi <address@hidden> writes:

> Hi, Michael,

Hi Zhongwei,

> This update includes following change:
> - Fix the auto completion bug.

Now that your FSF assignment is complete (congratulations!), I did a
more granular review + test of your patch. Some remarks:

- I still would like to call the entry in tramp-methods tramp-default-port.
  We shall not introduce different entries for the same thing, and
  tramp-default-port does exist already.

- I've added traces to tramp-adb-parse-device-names, and now it's
  visible that this is called very often. Not so good with performance
  in mind; maybe we could do something in the future to change it. Call
  it less often, cache the result, whatever.

  I have also changed it to return "host#port" entries. So we could work
  with this syntax internally. By this, I could remove
  tramp-adb-parse-device-names-for-completion.

- I have rewritten tramp-adb-get-host-for-execution (new name is
  tramp-adb-get-device) in order to simplify it. Works for me, but I'm
  not sure whether it is OK for all use cases. Please check!
 
I've checked the "try connect" feature, by visit: /adb:192.168.1.127#5555:, and find tramp will try to connect: 192.168.1.127#5555:5555, and end with failure. Do you have similar issue?

And the comment in tramp-adb-get-device function is not correct now, I think we can just remove them:
 ;; Checking whether exe-name is in devices. Follow cases are checked:
      ;;  - yes, correct exe-name returns.
      ;;  - not, true device name is: xxxx
      ;;    user types: xxxx:port-number. Error returns with tips of correct name
      ;;  - not, true device name is: xxxx:port-number
      ;;    user types: xxxx. Tramp will auto-complete port-number for user
      ;;  - not, no device name is similar as user gives. if
      ;;    tramp-adb-connect-if-not-connected is t, try to connect that device,
      ;;    else return error

  It uses now also caching. tramp-adb-search-host-in-devices is not
  needed anymore.

- In tramp-adb-maybe-open-connection I have applied changes resulting
  from the previous ones. I have also removed some checks for devices;
  hopefully I haven't removed too much.

Could you, please, test the updated patch? If it is fine with you, I
would commit it in your name. I would need ChangeLog entries from you.
 
What kind of ChangeLog should I provide? Is it similar like the commit message? Like:
   Enable adb access with port format, e.g. /adb:164.2.168.1#5555:/

> Best regards,
> Zhongwei

Best regards, Michael.

Thanks!
Zhongwei

reply via email to

[Prev in Thread] Current Thread [Next in Thread]