Fix for #506 impossible to filter in null values #507
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The fix for AllenFang#427 (AllenFang#428) is too aggressive in the sense that it prevents altogether to filter in a value of null. Whatever the input in the filter input, it's impossible to include the line (it returns false systematically to the filter function on a null value) I'd like to suggest a different fix. Instead of returning false, we set targetVal to the empty string in case the value is null or undefined. I believe this makes sense since the input from the user will be mostly text based (we could imagine custom filters for which it's not true) but in most cases it is. So assuming that having a value of null or undefined maps to an empty string seems sensible. With that change, it becomes possible to filter in null values using the regex filter for instance and specifying a regex that only allows an empty string.
|
@jfremy, I think it's good for fixing this issues :) Thanks 👍 |
|
@jfremy, released on |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The fix for #427 (#428) is too aggressive in the sense that it prevents altogether to filter in a value of null.
Whatever the input in the filter input, it's impossible to include the line (it returns false systematically to the filter function on a null value)
I'd like to suggest a different fix. Instead of returning false, we set targetVal to the empty string in case the value is null or undefined. I believe this makes sense since the input from the user will be mostly text based (we could imagine custom filters for which it's not true) but in most cases it is. So assuming that having a value of null or undefined maps to an empty string seems sensible.
With that change, it becomes possible to filter in null values using the regex filter for instance and specifying a regex that only allows an empty string.