monsanto / readline-complete.el

autocomplete in shell mode buffers
67 stars 16 forks source link

In newer development emacs versions, completion fails and deletes prompt #11

Closed ian-kelling closed 9 years ago

ian-kelling commented 10 years ago

It happens when doing a second completion on the same line as a first completion.

funcall: Wrong type argument: stringp, nil

And sometimes this message also.

Error running timer `ac-update-greedy': (wrong-type-argument stringp nil)

I'm hoping to investigate

It seemed to start happening at commit 76ef9e3 a few months ago in the git mirror of emacs dev sources, but I couldn't see changes that would create the problem. And from that commit it was happening on the first completion, then at some later commit, it was partially fixed so it only happened on the 2nd completion. I'm hoping to take some time to investigate and fix it in the next few days.

edoloughlin commented 9 years ago

Any update on this? I upgraded all my packages yesterday (without upgrading emacs) and I'm seeing this out of the blue on 24.4.1

ian-kelling commented 9 years ago

I haven't gotten around to spending the time to dive into it. Feel free to take the plunge if you want. I might get some time this weekend. I've been staying at the commit before this regression.

emelin commented 9 years ago

Hit into this too.

monsanto commented 9 years ago

I'm not actively maintaining readline-complete; if someone else wants to take a stab at this I can merge in the results.

ian-kelling commented 9 years ago

The commit I mentioned doesn't exist anymore since emacs moved to using git instead of just having a git mirror. But forget about that.

This a regression due to commit ccab2955dfeffba0635 in emacs. The problem is with some complicated logic changes to a very complicated very long c function, which I don't fully understand yet. Until I have a well understood fix, here is a fix which seems to work fine. Remove this from src/process.c around line 4657.

/* After reading input, vacuum up any leftovers without waiting.  */
if (0 <= got_some_input)
  nsecs = -1;

re maintenence of this package: readline-complete functionality is quite important to me, I will at least make sure it keeps working, and hoping to do more than that if I find the time.

monsanto commented 9 years ago

@ian-kelling If you have a minimal test case emacs-dev should be able to help you out with the C stuff. I think people are generally afraid to touch the C code which is why it is so long and hard to read. At least, that is what I was told when I found a bug that segfaulted Emacs and tried to navigate the rendering code on my own...

And thanks for the help. I may have some time in a few weeks to look at all of this myself.

ian-kelling commented 9 years ago

I've spent quite a bit more time on this, but from the elisp side. The commit I mentioned causes 'sleep-for' to stop sleeping as soon as emacs collects any output from subprocesses. This appears to be a bug. readline-complete tries to 'sleep-for' rlc-attempts times for rlc-timeout (30 & .3 for 1 second total) to get all the output from bash, but for some commands, the sleep-for gets woken up all 30 times in way way less than 1 second, reading only a small chunk of output each time it slept. readline-complete then gives up, but bash keeps outputing characters, usually they are ^H chars, aka backspace, which causes the prompt to be erased. I haven't looked into why exactly why the output is handled that way, but it's not a priority to make this work better since I'm not seeing any normal bash command which can do this (the output always includes something else and gets handled better). I am going to make a small test case and propose a patch to emacs-dev to revert the code change which caused this, which was supposed to be a minor optimization and is undocumented, except in the code itself, which I still don't understand, but emacs-dev will be able to help, and I'd bet it's the cause of some other bugs which no one has yet spent hours tracking down.

monsanto commented 9 years ago

@ian-kelling Great detective skills! Post a link to the mailing list thread?

ian-kelling commented 9 years ago

Yes, I will post a link when I post to the list. Hoping to post tomorrow. I figured out all the c code finally.

ian-kelling commented 9 years ago

An update on this. I was going over my solution and realized I was mistaken thinking it did not unfix the bug it was meant to fix so I've been working on a new fix. I'm almost done and planning to post it next weekend.

ian-kelling commented 9 years ago

Took a bit longer than expected, but I've posted the patches. prerequisite patch: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20976 http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20978

monsanto commented 9 years ago

:tada: :tada: :tada: Kind of surreal to see this issue on the road to being fixed. Thanks for all of your work, @ian-kelling!

ian-kelling commented 9 years ago

The patch accepted was accepted and merged! It really took a lot of work to get right, and I'm so glad it's done.