search

emacs-pe / docker-tramp.el

TRAMP integration for docker containers
340 stars 25 forks source link

Add fixed tramp-wait-for-output. Fixes #11 #12

Closed Tritlo closed 7 years ago

Tritlo commented 8 years ago

This pull request adds a re-definition for the tramp function, to fix issue #11 tramp-wait-for-output so that it recognizes outputs of shells with control sequences after them. This allow people to use docker-tramp with alpine containers, which I imagine is a common use case. This bugfix is of course not supposed to be here, but rather in tramp itself. However, getting something merged into tramp is a much larger undertaking. Thus I propose to add this to docker-tramp, since this is the only place I've actually seen this bug having an effect.

PuercoPop commented 8 years ago

Btw the indentation is off (for example the then and else forms of an are less indented than the if). To fix this just select the whole defun and then M-x indent-for-tab-command. With the default keybindings it is C-M- then

Tritlo commented 8 years ago

As I've said, this is a direct copy from tramp-sh.el, so I do not want to edit the function further.

Tritlo commented 8 years ago

You can see the original function here: https://github.com/emacs-mirror/emacs/blob/master/lisp/net/tramp-sh.el#L4992

Tritlo commented 8 years ago

Alright, I updated the comment as well.

marsam commented 8 years ago

I don't think is a good idea to shadow an internal tramp function. Given that this is going to be fixed in the next Emacs release, maybe a warning in the README should be enough?

Tritlo commented 8 years ago

Yes, that seems reasonable. You could however add a warning and suggest this as a fix, since adoptions of new versions is often slow.

marsam commented 7 years ago

Sorry for the late response. I added docker-tramp-compat.el with the patch for Tramp<2.3, and added a comment in the readme with instructions.