Skip to content

Better completion in region #1357

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

Merged
8 commits merged into from
Feb 2, 2016
Merged

Better completion in region #1357

8 commits merged into from
Feb 2, 2016

Conversation

thierryvolpiatto
Copy link
Member

No description provided.

Michael Heerdegen and others added 5 commits January 14, 2016 15:06
* helm-mode.el (helm--completion-in-region): Do it.
@issue-dispenser
Copy link
Contributor

Hi, thanks for this -- it fixed a bug I was having in haskell-mode.

Currently it also introduces a new bug -- if the string between start and end isn't the prefix of the completion, then I get [No matches].

For instance, when I did helm--completion-at-point in a Haskell repl with :cd I used to get all of the directories in the current directory, (which is what (helm--completion-at-point 313 313 '(<list of stuff>)) got me, with 313 being the cursor position).

Now it gives me [No matches].

* helm-mode.el (helm--completion-in-region): Do it.
@thierryvolpiatto
Copy link
Member Author

issue-dispenser notifications@github.com writes:

Hi, thanks for this -- it fixed a bug I was having in haskell-mode.

Currently it also introduces a new bug -- if the string between start and end isn't the prefix of the completion, then I get [No matches].

For instance, when I did helm--completion-at-point in a Haskell repl with :cd I used to get all of the directories in the current directory, (which is what
(helm--completion-at-point 313 313 '()) got me, with 313 being the cursor position).

Now it gives me [No matches].

Can't try with haskell but I have commited a fix that may work when you
try to :cd with your haskell repl.
Let us know if it works.

Thanks.

Thierry

* helm-mode.el (helm--completion-in-region): Do it.
@issue-dispenser
Copy link
Contributor

It appears like it's working now, thanks.

@thierryvolpiatto
Copy link
Member Author

@michael-heerdegen Perhaps this can be merged now ?

@ghost
Copy link

ghost commented Feb 1, 2016

Thierry Volpiatto notifications@github.com writes:

Perhaps this can be merged now ?

I still want to have a final look. I'll merge it tomorrow, ok?

@thierryvolpiatto
Copy link
Member Author

Michael Heerdegen notifications@github.com writes:

Thierry Volpiatto notifications@github.com writes:

Perhaps this can be merged now ?

I still want to have a final look. I'll merge it tomorrow, ok?

Ok great.

Thanks.

Thierry

@ghost ghost merged commit c8510e9 into master Feb 2, 2016
@ghost ghost deleted the better-completion-in-region branch February 2, 2016 16:54
@ghost
Copy link

ghost commented Feb 2, 2016

The only question that raised while I read the changes again was whether it could be worth it to honor the display-sort-function the completion metadata might include.

@thierryvolpiatto
Copy link
Member Author

Michael Heerdegen notifications@github.com writes:

The only question that raised while I read the changes again was
whether it could be worth it to honor the display-sort-function the
completion metadata might include.

We could add something like this in fc-transformer:

(append (list 'helm-cr-default-transformer)
        (list (helm-aif (completion-metadata-get metadata 'display-sort-function)
                  (lambda (candidates _source)
                    (funcall it candidates))
                (lambda (candidates _source)
                  (sort candidates 'helm-generic-sort-fn)))))

even if in practice display-sort-function seems rarely provided in
metadata's.

Thierry

@ghost
Copy link

ghost commented Feb 3, 2016

Would we have to care about the fact that candidates in helm can include conses (I guess we must)?

@ghost
Copy link

ghost commented Feb 3, 2016

But actually, it seems to be used nowhere in vanilla Emacs. So it's probably not worth it to support that feature.

@thierryvolpiatto
Copy link
Member Author

Michael Heerdegen notifications@github.com writes:

Would we have to care about the fact that candidates in helm can include conses (I guess we must)?

Oh yes, right, in a similar way I did in the generic sort fn.

But as you said, probably it is prematurate for now to add this, let
emacs provide some completions with this first.

Thierry

This pull request was closed.
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.

2 participants