terrycojones / daudin

A Python command-line shell
MIT License
176 stars 9 forks source link

Error decoding data from vim command #2

Open terrycojones opened 4 years ago

terrycojones commented 4 years ago

I ran vim in daudin to see how it would go. All went well until I exited.

>>> vim Makefile 
Traceback (most recent call last):
  File "/home/terry/s/me/daudin/daudinlib/interaction.py", line 58, in runCommand
    nCommands)
  File "/home/terry/s/me/daudin/daudinlib/pipeline.py", line 128, in run
    handled, doPrint = self._tryShell(fullCommand, print_)
  File "/home/terry/s/me/daudin/daudinlib/pipeline.py", line 238, in _tryShell
    result = self.sh(command, print_=print_)
  File "/home/terry/s/me/daudin/daudinlib/pipeline.py", line 283, in sh
    return run(stdin, *args, **kwargs)
  File "/home/terry/s/me/daudin/daudinlib/pipeline.py", line 374, in _shPty
    return ANSI_esc.sub('', result.decode('utf-8')).replace('\r\n', '\n')
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xbd in position 145: invalid start byte

I have a feeling we should just silently (unless debugging is on) swallow such errors. It would be good to know why this one happens and fix it, but OTOH we probably don't want all the data that vim writes to the terminal to be stored in the _ pipeline variable. But how to distinguish with cases we do want, e.g., when running ls?

rybot666 commented 4 years ago

@terrycojones Is it possible to dynamically detect output characters that move the cursor and assume from there?

terrycojones commented 4 years ago

Hi @rybot666 I'm not sure what you mean exactly. Do you mean that if an escape sequence is detected in the output then not keep the output from the command at all? The trouble is that sometimes you probably want the output stripped of the escape sequences (as in the case of e.g., ls --color=always or git status) whereas at other times you probably just want to throw all the output away (as with use of e.g., vim). Maybe there should be a helper command supplied which can strip escape sequences out of output. Thanks for commenting!

rybot666 commented 4 years ago

@terrycojones I mean only detect specific escape sequences (cursor moving) as only interactive programs use them. You could also have a syntax to "ignore this command" in pipes

john9631 commented 4 years ago

Something I noted.

If I enter vim from daudin then copy code for external use with ctrl c I crash with:

Vim: Caught deadly signal TERM Vim: Finished

Any chance that ctrl-c can be ignored while in another application?

rybot666 commented 4 years ago

@john9631 No terminal does this - control C is a well known keyboard code for "end this program". VIM has its own keybinds to copy, as does Emacs

john9631 commented 4 years ago

Ctrl C is non uncommonly mapped to copy for the external environment.

https://vim.fandom.com/wiki/Quick_yank_and_paste

If you haven't tried it @rybot666 I highly recommend you try ptpython, an enhanced python shell that integrates vim in both ptpython and ptipython modes.

terrycojones commented 4 years ago

Something I noted.

If I enter vim from daudin then copy code for external use with ctrl c I crash with:

Vim: Caught deadly signal TERM Vim: Finished

Any chance that ctrl-c can be ignored while in another application?

Hi @john9631 - thanks for commenting. Yes, this should be straightforward, and I guess there's no reason not to just catch & ignore Ctrl-C in general. Are you actually using daudin day-to-day?

Sorry for being a bit slow. I accidentally "updated" the firmware on my main laptop and now it only boots into Windows, not Linux. Will hopefully have that sorted soon... and then can get back to doing more useful things!

john9631 commented 4 years ago

Thanks @terrycojones, yes I have been using it most days although I did alias it to psh.

terrycojones commented 4 years ago

Thanks @terrycojones, yes I have been using it most days although I did alias it to psh.

Great! :-)

I have just pushed a new version 0.0.17 which should ignore control-c in interactive use. Thanks for posting the issue.

terrycojones commented 4 years ago

@john9631 No terminal does this - control C is a well known keyboard code for "end this program". VIM has its own keybinds to copy, as does Emacs

The new version is now ignoring SIGINT. So if your terminal happens to send that signal when control-c is pressed, it will be ignored. If it pastes text from the clipboard, that should work fine. If a SIGINT comes from elsewhere (e.g., a kill command from some other command line) it will also be ignored.

terrycojones commented 4 years ago

@terrycojones Is it possible to dynamically detect output characters that move the cursor and assume from there?

@rybot666 The latest code detects a UnicodeDecodeError and sets the UNIX command's output to the empty string. An alternative would be to just put the bytes into the return value. At least for now the ugly error is silenced (you can still see it with debug / traceback on).

terrycojones commented 4 years ago

Thanks @terrycojones, yes I have been using it most days although I did alias it to psh.

Great! :-)

I have just pushed a new version 0.0.17 which should ignore control-c in interactive use. Thanks for posting the issue.

Hmm, I spoke too soon! I'm going to have to look into this a bit more. The vim is running in a pseudo-tty and actually I'm detecting the control-c on stdin and then calling a Python subprocess method to terminate the process (hence the SIGTERM not a SIGINT). That's obviously wrong (especially considering @rybot666's comment that control-c isn't necessarily bound to send an interrupt). daudin should just let the default thing happen (whatever that is). I think I put that code in to handle control-c pretty early (before I started using a pseudo-tty) when I realized I couldn't interrupt a launched sub-process, but maybe it's no longer needed. I'll have a play...

terrycojones commented 4 years ago

OK, just pushed version 0.0.18 which does something more sensible - passes a SIGINT along to the pseudotty subprocess. So vim gets a Control-C and prints a message whereas a ping gets interrupted, etc. Thanks @john9631 and @rybot666. See how it is for you, if you have time & interest.

john9631 commented 4 years ago

Working fine here. Exactly as expected. I see what you mean about psh being gone too - namespace pollution - so I've changed my mapping to dsh for daudin adding to sh, dash, bash and zsh currently on the machine to my knowledge.