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

ivy.el: Remove webjump completing-read handler #1802

Closed
wants to merge 1 commit into from

Conversation

basil-conto
Copy link
Collaborator

@basil-conto basil-conto commented Nov 15, 2018

(ivy-completing-read-handlers-alist): Do not pass empty string as DEF argument to completing-read in webjump command, as this adds a bogus empty candidate to the webjump-sites collection.

Re: #1049
Cc: @DarwinAwardWinner

(ivy-completing-read-handlers-alist): Do not pass empty string as
DEF argument to completing-read in webjump command, as this adds a
bogus empty candidate to the webjump-sites collection.

Re: abo-abo#1049
@DarwinAwardWinner
Copy link
Contributor

@DarwinAwardWinner DarwinAwardWinner commented Nov 15, 2018

I'm not personally a user of webjump, so I can't comment on whether it actually needs an empty string default or not. It may have been updated at some point so that it no longer requires the workaround.

@basil-conto
Copy link
Collaborator Author

@basil-conto basil-conto commented Nov 15, 2018

@DarwinAwardWinner Neither webjump.el nor the underlying browse-url.el have been changed in this respect since at least before Emacs 24. When webjump completion on the webjump-sites collection exits with no user input, webjump simply passes the empty string on to browse-url. This behaviour is constant regardless of whether DEF is nil or "". The only effect achieved by using ivy-completing-read-with-empty-string-def as the handler is that Ivy adds a bogus empty candidate to the front of the collection, to no avail.

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Nov 18, 2018

Thanks.

@basil-conto basil-conto deleted the blc/webjump branch Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants