Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`insert-char` #38

Closed
dudebout opened this issue Oct 2, 2013 · 17 comments
Closed

`insert-char` #38

dudebout opened this issue Oct 2, 2013 · 17 comments

Comments

@dudebout
Copy link

@dudebout dudebout commented Oct 2, 2013

Would it be possible to have ido work for insert-char. I could not figure out why it was not working by default.

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 2, 2013

Can you give an example where you get the undesired behavior?

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 2, 2013

I was just trying to use C-x 8 <ret> and expected ido to kick in but I get the old completion mechanism.

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 2, 2013

Ok, this is because read-char-by-name passes a function for the collection argument of completing-read. You can try adding a function override to it and see if it works.

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 2, 2013

Sorry to ask, but where did you find this info?

On Wed, Oct 2, 2013 at 6:03 PM, Ryan Thompson notifications@github.comwrote:

Ok, this is because read-char-by-name passes a function for the
collection argument of completing-read. You can try adding a function
override to it and see if it works.


Reply to this email directly or view it on GitHubhttps://github.com//issues/38#issuecomment-25581059
.

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 2, 2013

I read the source of insert-char (M-x find-function insert-char) and found the function call within it that seemed most likely to lead to a call to completing-read, which was read-char-by-name. Then I did M-x find-function read-char-by-name to find that function's definition, and saw that it made a call to completing read with a lambda as the collection argument.

This is somewhat complicated by the fact that insert-char is a builtin function implemented in C, so you have to have the emacs source lying around in order to view its definition and you need to set find-function-C-source-directory appropriately.

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 2, 2013

Ok. Thanks. I need to start looking into the C code one of these days.

On Wednesday, October 2, 2013, Ryan Thompson wrote:

I read the source of insert-char (M-x find-function insert-char) and
found the function call within it that seemed most likely to lead to a call
to completing-read, which was read-char-by-name. Then I did M-x
find-function read-char-by-name to find that function's definition, and
saw that it made a call to completing read with a lambda as the collection
argument.


Reply to this email directly or view it on GitHubhttps://github.com//issues/38#issuecomment-25582178
.

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 2, 2013

I started to look into fixing the issue and realized you had already commited a fix. Thanks

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 2, 2013

Now that it is fixed, I can tell with confidence that this used to work before today. I clearly remember using this interface. What had changed to cause the regression?

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 2, 2013

ido-ubiquitous 1.x was happy to use ido any time the collection argument was a function. However, this broke a bunch of cases where the functions would do non-standard methods of completion. There's no way to tell which functions are ok and which are not, so I erred on the side of not breaking things and changed it to avoid ido when collection is a function, unless an override enables it.

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 2, 2013

OK. This explains the commit fix as well. Thanks for the prompt handling. Much appreciated. If I find other cases I will be in a better position to help you fix them.

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 3, 2013

Here are the relevant commits:

cc60675 * Allow overrides to force ido when collection is a function
10ea202 * Implement whitelist for collection functions
072bb18 * Correctly disable ido when collection is a function

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 3, 2013

Hopefully I will reach a point where everything can be fixed just by adding an override.

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 3, 2013

This change explains why ido stopped working in magit as well. However in the magit case, there is an option to reenable it: (setq magit-completing-read-function 'magit-ido-completing-read)

It makes me feel better to understand what was going on. Thanks

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 4, 2013

Yes, I added exceptions for magit and org-mode since they already have ido options: 0e966a2

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 4, 2013

Maybe you could put this info in the readme. It is not obvious that ubiquitous does not work everywhere anymore.

@DarwinAwardWinner

This comment has been minimized.

Copy link
Owner

@DarwinAwardWinner DarwinAwardWinner commented Oct 4, 2013

@dudebout

This comment has been minimized.

Copy link
Author

@dudebout dudebout commented Oct 4, 2013

Awesome. Thanks.

On Thursday, October 3, 2013, Ryan Thompson wrote:

47ceb4047ceb40


Reply to this email directly or view it on GitHubhttps://github.com//issues/38#issuecomment-25673723
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.