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

Can.string getNext function fix #340

Merged
merged 3 commits into from Apr 16, 2013

Conversation

Projects
None yet
2 participants
@schovi
Contributor

schovi commented Mar 31, 2013

Fix getNext function in can.string component
There was wrong logic behind evaluation if add or not add empty object

// Before
getNext = function( obj, prop, add ) {
            return prop in obj ?
                obj[ prop ] : 
                ( add && ( obj[ prop ] = {} ));
        }

getNext({}, 'foo', undefined) // -> undefined // this is ok
getNext({}, 'foo', false) // -> false // instead of undefined
getNext({}, 'foo', "") // -> "" //instead of undefined
getNext({}, 'foo', true) // -> {} // which is OK
etc....

// ------------------
//After
getNext = function( obj, prop, add ) {
      var result = obj[prop];

      if(result === undefined && add === true) { result = obj[prop] = {} }
      return result
    }

getNext({}, 'foo', undefined) // -> undefined
getNext({}, 'foo', false) // -> undefined
getNext({}, 'foo', "") // -> undefined
var obj = {}
getNext(obj, 'foo', true) // -> {} // and correctly obj = {foo: {}} 
@schovi

This comment has been minimized.

Show comment
Hide comment
@schovi

schovi Mar 31, 2013

Contributor

This partitionaly solve #339

Contributor

schovi commented Mar 31, 2013

This partitionaly solve #339

@schovi

This comment has been minimized.

Show comment
Hide comment
@schovi

schovi Mar 31, 2013

Contributor

This also fix following. Last commit add test for this.

BEFORE
can.sub("a:{a}",{}) // -> null
can.sub("a:{a}",{}, true) // -> "a:false" // Which can break some logic

AFTER
can.sub("a:{a}",{}, true) // -> null
Contributor

schovi commented Mar 31, 2013

This also fix following. Last commit add test for this.

BEFORE
can.sub("a:{a}",{}) // -> null
can.sub("a:{a}",{}, true) // -> "a:false" // Which can break some logic

AFTER
can.sub("a:{a}",{}, true) // -> null

daffl added a commit that referenced this pull request Apr 16, 2013

Merge pull request #340 from schovi/can-getNext-fix
Can.string getNext function fix

@daffl daffl merged commit ff1d361 into canjs:master Apr 16, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment