search

creichert / ido-vertical-mode.el

makes ido-mode display vertically
211 stars 22 forks source link

Implement customizable indicator (#28) #43

Closed randymorris closed 8 years ago

randymorris commented 8 years ago

I'm not familiar with ert so I haven't run or updated the tests.

I waffled on whether or not something should be added to the docstring of ido-vertical-indicator about the length of the string but in the end I left it out. I couldn't immediately find anything to require a specific length on a string type in customize.

randymorris commented 8 years ago

This is entirely broken when I run it with emacs -q. I need to investigate more. As of now this pull request is pretty much invalid.

creichert commented 8 years ago

Thanks for the PR. The tests "should" just run when you push the PR so you can validate them by checking the travis output.

If you end up fixing this, I'll chime in to make sure those builds are running so you don't have to set it up locally.

randymorris commented 8 years ago

I figured out how I could run them locally. This command from the root of the git repo launches emacs and runs the tests:

emacs --eval "(add-to-list 'load-path \"$PWD\")" -l test/ido-vertical-mode-test.el --eval "(ert t)"

It might be worth documenting whatever your suggested method of running the tests is so it's more obvious how to test things before creating a pull request (unless I've just missed it, in which case I apologize).

It turns out I was being very novel in my approach and I have no idea why it was working for me in the first place. I had completely forgotten that this package is mostly just a facade on top of ido and what I had done was breaking ido for some reason. When I get some more time I'll look into it again. Sorry for all the noise.