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

Slightly change score strategy for columns #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hiddenmemory
Copy link

Previously, if a column had no match, it would short circuit returning None, stopping subsequent columns from potentially matching.

This change means that non-matching columns are scored as zero, allowing subsequent columns to be matched against.

If the match has zero score after all columns, then we return None, retaining previous behaviour.

All existing tests pass.

Previously, if a column had no match, it would short circuit
returning None.

This means that if you have a match term on a
second column, but the first didn't match, you wouldn't get any
match for the row.

This change means that non-matching columns are scored as zero,
and then if the match has zero score after all columns, then we
return zero.
@pascalkuthe
Copy link
Member

this is not how columns are intended to be used. It's an AND match not an OR match. For example in helix diagnostic picker %severity ERR %message type will show you all type errors. With this change that would not work anymore.

@alexrutar
Copy link
Contributor

Would it be worth permitting more general (user-provided) column score strategies? Something like making the MultiPattern::score function generic over a "score strategy" trait.

This would handle the comment in the codebase about potentially allowing column weights.

Then, in principle, as long as the "score strategy" trait is sufficiently general, someone could implement the behaviour suggested in this PR if desired.

@pascalkuthe
Copy link
Member

I would be more option to that but I think the better way to do this would be to add general OR predicate (the one aspect of FZF we haven't ported yet) and then we could have a general mechanism to include OR patterns that would also apply to columns (like colum1: foo & column2: bar | colum3: baz).

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.

3 participants