Gene Goykhman <g...@indigo1.com> writes:
> Hi Michael,
Hi Gene,
>> Well, you disable this for all completions. I don't know whether we want
>> this. It is useful for the very first hop, and not only for the docker
>> method, but for all methods.
>
> I agree, and it doesn't seem that TRAMP currently allows that level of
> granularity for disabling caching. It does seem out of scope for this
> particular enhancement.
Indeed. Perhaps we can leave it out for now. I also don't believe that
we need to disable completion from the cache at all. If we would be able
to determine, which cache information is valid where, we could use this
information. For example, "host1" is reachable via ssh only from the
local host, and "host2" is reachable via ssh only from "host1". "host2"
would be a completion candidate only after a first hop to "host1".
But we don't have this information yet. Let's do this later.
>> > (add-to-list 'tramp-completion-function-alist
>> > `("docker"
>> > (tramp-container--completion-function ,tramp-docker-program)
>> > (my-tramp-container-completion-function
>> > ,tramp-docker-program)))
>
>> So you have two entries. Do you still need
>> tramp-container--completion-function?
>
> I wasn't sure whether it was important to keep the original function in the
> list.
It depends. If the new implementation in
my-tramp-container-completion-function covers also the case of
tramp-container--completion-function, then we need only one function. I
would propose to do so, and add all changes into the latter function.
Btw, it shouldn't work only for docker, but also for podman.
>> > (defun my-tramp-docker-host-path (orig)
>> > ;; Strips off "|docker" suffix from ORIG allowing
>> > ;; it to be converted to a tramp-file using TRAMP's path functions.
>> > (let ((regexp "\\(.*\\)|docker"))
>> > (if (string-match regexp orig)
>> > (concat (match-string 1 orig) ":/")
>> > orig)))
>
>> Well, this is docker related. The proper Tramp functions would be
>
>> (tramp-make-tramp-file-name (tramp-dissect-hop-name
>> (tramp-file-name-hop (tramp-dissect-file-name FILENAME))))
>
>> Hmm, perhaps this is worth an own function. Or perhaps we have it
>> already, and I don't remember. We have a similar logic in computing the
>> previous hop in function tramp-compute-multi-hops, but not in the same
>> sense as we need it here.
>
>> And this also does not handle the case of more than two hops, like in
>> "/ssh:host1|ssh:host2|docker:container::"
>
> Yes I have to defer to your judgement here. It was a very simple
> attempt that worked for my particular use-case.
We could start with just one hop. But we shouldn't hard-code "docker"
and alike, in order to be open for other methods.
>> As said, I propose to integrate it in
>> tramp-completion-handle-file-name-all-completions.
>> And I don't understand, why you call (tramp-make-tramp-file-name
>> (tramp-dissect-file-name ...))
>> my-host-path is already a proper file name, isn't it?
>
> They are slightly different. For example, testing in my sutation I have this
> my-host-path:
>
> /ssh:in...@line5.timetiger.com|sudo:line5.timetiger.com:/
>
> But after the tramp-make-tramp-file-name transformation it is:
>
> /ssh:in...@line5.timetiger.com|sudo:r...@line5.timetiger.com:/
>
> I think the additional username after sudo: is needed in order to execute
> docker on the remote host. I'm not sure if this is the best way to accomplish
> this.
/sudo:line5.timetiger.com:/ and /sudo:r...@line5.timetiger.com:/ are
identical in terms of remote file names. The user name is just expanded
due to the respective entry in tramp-default-user-alist (the second entry):
--8<---------------cut here---------------start------------->8---
tramp-default-user-alist is a variable defined in ‘tramp.el’.
Its value is
(("\\`\\(?:fcp\\|krlogin\\|nc\\|r\\(?:cp\\|emcp\\|sh\\)\\|telnet\\)\\'"
nil "albinus")
("\\`\\(?:doas\\|ksu\\|su\\(?:do\\)?\\)\\'" nil #1="root")
("\\`smb\\'" nil nil) ("\\`sudoedit\\'" nil #1#))
Original value was nil
--8<---------------cut here---------------end--------------->8---
I propose you try to write patches towards the existing Tramp. We can
discuss them, and we can add them once your FSF legal papers are fine,
this shouldn't last longer than two weeks I hope.
If you want to check your changes regularly, you might consider
tramp-tests.el There are already two test cases for completion, but
nothing for multi-hop completion yet. I find it always useful to write
tests in parallel to new features, in order to be sure nothing
breaks. And also in order to demonstrate how it is intended to work.
Best regards, Michael.
