For Selectrum, add commands that toggle filtering methods. #72

okamsn · 2020-10-12T22:37:07Z

While using Selectrum, I sometimes want to change what filtering method the current search is using, similar to Isearch's isearch-toggle-* commands. This pull request adds commands for this to selectrum-prescient.el.

Currently, this toggling only affects the running Selectrum buffer. Using a prefix argument with a command locally sets prescient-filter-method to only use the method corresponding to that command.

Is there anything that you would like changed?

raxod502

Thank you, this looks great! Some comments.

raxod502 · 2020-10-17T16:16:40Z

README.md

@@ -64,7 +64,7 @@ ones, and then the remaining candidates are sorted by length. If you
 don't like the algorithm used for filtering, you can choose a
 different one by customizing `prescient-filter-method`.

-## Configuration
+## Configuration and Other Features


Suggested change

## Configuration and Other Features

## Configuration and other features

raxod502 · 2020-10-17T16:16:45Z

README.md

@@ -90,6 +90,9 @@ different one by customizing `prescient-filter-method`.
  matching, regexp matching, fuzzy matching, prefix matching, or any
  combination of those. See the docstring for full details.

+### Ivy-Specific


Suggested change

### Ivy-Specific

### Ivy-specific

raxod502 · 2020-10-17T16:16:49Z

README.md

@@ -110,6 +113,33 @@ different one by customizing `prescient-filter-method`.
  the Ivy documentation for information on how Ivy sorts by default,
  and how to customize it manually.

+### Selectrum-Specific


Suggested change

### Selectrum-Specific

### Selectrum-specific

raxod502 · 2020-10-17T16:16:58Z

README.md

@@ -90,6 +90,9 @@ different one by customizing `prescient-filter-method`.
  matching, regexp matching, fuzzy matching, prefix matching, or any
  combination of those. See the docstring for full details.

+### Ivy-Specific
+The following user options are specific to using Prescient with Ivy:


Suggested change

The following user options are specific to using Prescient with Ivy:

The following user options are specific to using `prescient.el` with Ivy:

raxod502 · 2020-10-17T16:18:16Z

selectrum-prescient.el

+The toggling of commands is temporary and does not affect the
+default filtering settings determined by `prescient-filter-method'.")
+;; Create a binding similar to the Isearch toggles.
+(defvar selectrum-minibuffer-map)


These defvar / declare-function forms can go in stub/selectrum.el with the others, so they're all centralized.

raxod502 · 2020-10-17T16:18:39Z

selectrum-prescient.el

+;; Create a binding similar to the Isearch toggles.
+(defvar selectrum-minibuffer-map)
+(define-key selectrum-minibuffer-map
+  "\M-s" selectrum-prescient-filter-toggle-map)


Suggested change

"\M-s" selectrum-prescient-filter-toggle-map)

(kbd "M-s") selectrum-prescient-filter-toggle-map)

Is it possible to make it so that this binding is only activated when selectrum-prescient-mode is enabled? While it's easy to do it this way, it's generally considered bad practice for simply loading a package to cause behavior changes.

I'd be happy to spend some time thinking about a good way of making this happen, if you're not sure.

raxod502 · 2020-10-17T16:20:50Z

selectrum-prescient.el

+                         (cons (quote ,filter-type)
+                               prescient-filter-method))
+             (message "%s filter toggled on."
+                      ,(capitalize filter-type-name))))


Would it be a good idea to report the new value of prescient-filter-method after it's updated? It might be hard to keep track of which methods have been toggled on and off if there are multiple.

raxod502 · 2020-10-17T16:23:38Z

README.md

+| M-s i | selectrum-prescient-toggle-initialism |
+| M-s l | selectrum-prescient-toggle-literal    |
+| M-s p | selectrum-prescient-toggle-prefix     |
+| M-s r | selectrum-prescient-toggle-regexp     |


Perhaps put these table entries in backticks, for consistency?

…t-mode.

…o toggle-commands Merge new anchor filtering method.

…method.

…oggle-map. This seems more appropriate, since the names of the commands are of the form "selectrum-prescient-toggle-*".

…ocal.

okamsn · 2020-10-18T02:03:15Z

I have tried to address what you wrote. Where else do you think things could be improved?

okamsn · 2020-10-21T14:53:34Z

@raxod502 Do you think it makes sense to add commands for toggling case and character folding buffer-locally? They're not Prescient features, but fit the general idea of the pull request.

raxod502 · 2020-10-23T14:04:40Z

Where else do you think things could be improved?

It looks great to me!

Do you think it makes sense to add commands for toggling case and character folding buffer-locally?

I think those probably make more sense to add on the Selectrum side, cc @clemera. They would presumably be useful even to people who don't use prescient.el.

Feel free to change anything else you like, otherwise I'm happy to merge this.

clemera · 2020-10-23T15:24:25Z

They would presumably be useful even to people who don't use prescient.el

Yes, that sounds like a great feature! For orderless and other filter functions which use C completion functions internally we would need to set completion-ignore-case in addition to case-fold-search. I'm not sure ab out char folding as I'm not very familiar with it but it seems all that would be necessary would be to transform the input using char-fold-to-regexp before passing it to the filter function.

okamsn · 2020-10-24T02:55:48Z

I think I am happy with this.

I don't have a use case for toggling the character folding, but I mentioned it because prescient.el uses it automatically for the literal filter.

raxod502 · 2020-10-26T14:12:52Z

I mentioned it because prescient.el uses it automatically for the literal filter.

You're right. This definitely complicates my idea above to just have the completion framework handle this kind of thing. Some work is required to figure out what the right interface between the two packages is. If somebody is interested in this feature, then I can see if I can come up with an idea.

raxod502

✨

okamsn and others added 12 commits October 12, 2020 17:43

Create commands for toggling Prescient filter methods in Selectrum.

0e8a5b7

Add declarations, and cut back on long lines.

49c92bd

With ARG, enable the filter method and disable all others.

69bfb9e

Add the toggle commands to README.md and CHANGELOG.md.

fd1fda2

Rename macro create-toggle-commands to create-and-bind-toggle-command.

b1a31a6

Add link to PR to changelog.

4eef38f

Fix warning about long lines.

6314a7b

In declare-function, prefix "selectrum" with "ext:".

2ce9753

Fill paragraphs in comments.

46fa7c7

Use "when nlistp" instead of "unless listp".

f85e285

Improve description of commands in README.md.

c5bd296

Improve descriptions in CHANGELOG.md.

4ea1323

okamsn force-pushed the toggle-commands branch from f3abe1d to 4ea1323 Compare October 15, 2020 01:32

Fix repetition of word "filtering" in README.md.

70bd7cc

raxod502 reviewed Oct 17, 2020

View reviewed changes

raxod502 added the waiting on response label Oct 17, 2020

okamsn added 14 commits October 17, 2020 15:19

Fix case in markdown titles.

068be33

Use "prescient.el" instead of "Prescient".

e3d3cc1

Use backticks in table of commands.

c5496e8

Move some defvar and declare-function uses to stub/selectrum.el.

ec26fea

Only bind selectrum-prescient-filter-toggle-map in selectrum-prescien…

532cdf7

…t-mode.

Merge branch 'master' of https://github.com/raxod502/prescient.el int…

c491a08

…o toggle-commands Merge new anchor filtering method.

Add binding for anchored filtering.

e120c56

Update comments. Signal user error when trying to remove only active …

d4f9c19

…method.

fixup! Add binding for anchored filtering.

b3eddc9

Update how filter list messaged.

e4d8b54

Try to improve wording in README.md.

2061803

Rename selectrum-prescient-filter-toggle-map to selectrum-prescient-t…

ed860e8

…oggle-map. This seems more appropriate, since the names of the commands are of the form "selectrum-prescient-toggle-*".

Add "is" to message about prescient-filter-method.

f3e7cba

Try to improve messaging.

80c5219

okamsn added 10 commits October 17, 2020 18:48

Use remq instead of remove when dealing with symbols.

d5dae93

Consolidate some code.

2f11ce1

Improve strings.

51f8317

Improve quoting.

80c63e8

Use symbolp instead of nlistp, since it's more specific.

c7cab1f

Formatting.

7133009

Remove use of progn, since not needed.

ee30e10

Messaging should be outside of if-body.

82e4e75

Use delq instead of remq, since it is known that variable is buffer-l…

db9e0ee

…ocal.

Re-add word "active" to user error message.

6a0ddc1

Improve wording in README and CHANGELOG.

1d810a5

Further backtickify

60eb5e2

okamsn added 4 commits October 23, 2020 20:42

Use backticks around keys in CHANGELOG.

c473da1

Improve wording in changelog, copying phrases from readme.

3b7dbcc

Improve wording.

b155aa8

Don't end sentences with two spaces.

309caee

okamsn force-pushed the toggle-commands branch from 37c5d1b to 309caee Compare October 24, 2020 02:50

okamsn mentioned this pull request Oct 24, 2020

Commands similar to Isearch toggles radian-software/selectrum#217

Closed

raxod502 approved these changes Oct 26, 2020

View reviewed changes

raxod502 merged commit 41443e1 into radian-software:master Oct 26, 2020

raxod502 removed the waiting on response label Oct 26, 2020

okamsn deleted the toggle-commands branch October 26, 2020 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For Selectrum, add commands that toggle filtering methods. #72

For Selectrum, add commands that toggle filtering methods. #72

okamsn commented Oct 12, 2020

raxod502 left a comment

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

raxod502 Oct 17, 2020

okamsn commented Oct 18, 2020

okamsn commented Oct 21, 2020

raxod502 commented Oct 23, 2020

clemera commented Oct 23, 2020

okamsn commented Oct 24, 2020 •

edited

Loading

raxod502 commented Oct 26, 2020

raxod502 left a comment

	## Configuration and Other Features
	## Configuration and other features

	The following user options are specific to using Prescient with Ivy:
	The following user options are specific to using `prescient.el` with Ivy:

	"\M-s" selectrum-prescient-filter-toggle-map)
	(kbd "M-s") selectrum-prescient-filter-toggle-map)

For Selectrum, add commands that toggle filtering methods. #72

For Selectrum, add commands that toggle filtering methods. #72

Conversation

okamsn commented Oct 12, 2020

raxod502 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okamsn commented Oct 18, 2020

okamsn commented Oct 21, 2020

raxod502 commented Oct 23, 2020

clemera commented Oct 23, 2020

okamsn commented Oct 24, 2020 • edited Loading

raxod502 commented Oct 26, 2020

raxod502 left a comment

Choose a reason for hiding this comment

okamsn commented Oct 24, 2020 •

edited

Loading