monsanto / readline-complete.el

autocomplete in shell mode buffers
67 stars 16 forks source link

Fix non-file candidates containing path characters #9

Closed rdallasgray closed 10 years ago

rdallasgray commented 10 years ago

This fixes an issue which manifests when completing non-file prefixes which contain slashes (e.g. "origin/master").

The present code will complete "origin/ma" as "origin/origin/master" (the candidate retains the full path); this selectively unpaths candidates if the full prefix (back to most recent whitespace) contains a slash.

monsanto commented 10 years ago

Thanks for the PR. Unfortunately I'll be very busy for the next week, but after that I will try to make time to review your work. If you don't hear from me in a week feel free to leave me a reminder note.

Talking to myself here, but TBH we should probably have a test suite for catching bugs. Ideally we could actually call readline, but a simpler alternative might be to just embed 'representative' readline responses in the test suite. It's not ideal because during development I remember encountering problems with the timing of responses. But it would be a start.

rdallasgray commented 10 years ago

Yep, sorry about that. Updated.

Re tests, I've had good mileage with https://github.com/rejeep/ert-runner.el. I'd suggest a suite of unit tests mocking the readline response, to test the logic around returning prefixes and candidates, and one or two integration tests which actually call readline, to catch non-functional issues.

Can I also suggest you set the project up with Cask (https://github.com/cask/cask)? It'd help with packaging and testing, and provide a route to getting the project onto Melpa (http://melpa.milkbox.net). I can probably put together a quick setup if it's something you'd be open to.

rdallasgray commented 10 years ago

Discovered an error in this so closing until fixed.