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

make Ranger return interface{}, not reflect.Value #112

Merged
merged 6 commits into from
Jan 3, 2020
Merged

make Ranger return interface{}, not reflect.Value #112

merged 6 commits into from
Jan 3, 2020

Conversation

sauerbraten
Copy link
Collaborator

Previously, the key/index and element values returned by Ranger were reflect.Values pointing to the underlying data (see https://golang.org/src/reflect/value.go?s=67137:67170#L2242 and https://golang.org/src/reflect/value.go#L140). This meant you could not compare e.g. elements of a string slice to a string literal. Also, storing a copy of a certain slice element's index and using it outside of the range did not work, since the copy only pointed to the actual index (which had the value len+1 after the range).

@sauerbraten
Copy link
Collaborator Author

21e5963 fixes a bug in how isset() evaluated field access expressions: because of variable shadowing in a loop (value := getFieldOrMethodValue(node.Field[i], value)), all fields were accessed on the base instead of the preceding field. This meant accessing a first-level field worked as expected, but a second-level field only worked when a field with the same name also happened to exist on the base of the access expression.

Similar code without this bug existed in evalBaseExpressionGroup(). To prevent the two implementations from diverging, the code was moved into a new evalFieldAccessExpression() function. In isSet(), the error is ignored so as to not stop template execution when visiting a non-existent field.

The test "IsSetExpression_3" added to eval_test.go failed before this fix.

@annismckenzie
Copy link
Member

What a long time it's been. 😞 These changes make sense. Because they change the Ranger interface, I'll have to bump the version to v3 (and add go modules along the way).

@sauerbraten
Copy link
Collaborator Author

I saw I pushed a few more changes to the branch later, which are unrelated to this PR. Most notably, the behavior of isset changes to return false when a variable is nil, even if set explicitly to that value.

@annismckenzie
Copy link
Member

I'll merge this, thanks! I'll fix the docs and examples using the Ranger interface in a separate pull request.

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