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

Fix for #2274 #2284

Merged
merged 2 commits into from Feb 16, 2016
Merged

Fix for #2274 #2284

merged 2 commits into from Feb 16, 2016

Conversation

mklauber
Copy link
Contributor

List fields (such as tags) when evaluated to produce tiddlers result in empty arrays. Using the exact not equals, an empty array is not the same as an empty string. By using equivelent not equals, we state that the field is either != "" or anything that can be coerced to "". Which, based on https://dorey.github.io/JavaScript-Equality-Table/ is false 0 [] or [[]] neither false nor 0 can be set as a tiddler field as both will end up being quoted ("false", "0") so this should work.

List fields (such as tags) when evaluated to produce tiddlers result in empty arrays.  Using the exact not equals, an empty array is not the same as an empty string.  By using equivelent not equals, we state that the field is either != "" or anything that can be coerced to "".  Which, based on https://dorey.github.io/JavaScript-Equality-Table/ is `false` `0` `[]` or `[[]]`` neither `false` nor `0` can be set as a tiddler field as both will end up being quoted (`"false"`, `"0"`) so this should work.
@Jermolene
Copy link
Owner

Hi @mklauber the semantics of the equivalent comparison operators can be surprising. For example:

> [] == ""
true // OK
> [""] == ""
true // Problematic
> ["",""] == ""
false // Inconsistent with the above

That's why the core pretty much exclusively uses the exact comparison operators. Here, I think it would be better to explicitly check for an array of length zero, alongside the existing check for an empty string.

@mklauber
Copy link
Contributor Author

Fixed that. I was possibly a little verbose in how I checked things, but I figured we wanted to be sure we were dealing with an array, before we checked its length.

@Jermolene
Copy link
Owner

Thanks @mklauber

Jermolene added a commit that referenced this pull request Feb 16, 2016
@Jermolene Jermolene merged commit 1f4df88 into Jermolene:master Feb 16, 2016
@Jermolene
Copy link
Owner

Hi @mklauber I think there's still a problem: the check for tiddler.fields[operator.operand] !== "" will succeed for an empty tags field. I guess we need to guard with a check for the field type being a string.

@mklauber
Copy link
Contributor Author

@Jermolene, I don't think this is an issue, unless something has recently changed. When I pause the debugger at the time when we hit this line, tiddler.fields.tags = Array[0].

@Jermolene
Copy link
Owner

In my simple testing I found that #2274 wasn't fixed by this; I only eyeballed the code, and didn't debug it properly.

@mklauber
Copy link
Contributor Author

Oops, I miss understood what you meant. I understand now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants