Return the css property value range in getInfoAtPos() api.

[RaymondLim]

This is required for CSS Shapes Editor to handle CSS property values spanning over multiple lines.

@JeffryBooher and @oslego Can you review this?

[RaymondLim]

@JeffryBooher I'm done fixing unit tests. Ready for you to land in master. I reviewed yours already. Thanks.

[RaymondLim]

Typo: currect

[JeffryBooher]

why the conditional? can't isNewItem just just be assigned to the argument coming in? you could do isNewItem: (isNewItem === true) which is more performant since it's just a simple math eval

[JeffryBooher]

Done with review. @RaymondLim let me know when you're ready for me to merge

[RaymondLim]

@JeffryBooher Changes are pushed. Ready for re-review.

[JeffryBooher]

I think that this might be better while (TokenUtils.moveNextToken(startCtx) && !startCtx.token.string.trim()); but I'm not sure if crockford will like it.

another thing that i'm wondering if TokenUtils.moveNextToken() returns false then we fall through and we're using the property name as the thing returned. Perhaps that needs a unit test in the case of a malformed property at the end of the file.

[RaymondLim]

Yes, linter will complain on the empty block. And you're mistaken with the direction. We're scanning forwards and not backwards here.

[JeffryBooher]

Looks good. Unit tests pass. Review Complete all comments addressed. Merging

[redmunds]

@JeffryBooher @RaymondLim I know this is already merged, but you should never use trim() in a condition. It unnecessarily creates a new string. Instead, use:

    if (startCtx.token.string.search(/\S/)) {

This detects the presence of a non-whitespace character.

[JeffryBooher]

@redmunds that's a good point about unnecessarily creating a new string but we aren't searching for the presence of whitespace, we're effectively looking for a string that is empty after removing all whitespace ...

[redmunds]

@JeffryBooher I believe that is the same thing -- let me know if you can think of any case that this doesn't work for.

[JeffryBooher]

@redmunds I think you could match on this: /[^\S]/ to search for any non-whitespace

[redmunds]

@JeffryBooher \s (lowercase) is for whitespace and \S (uppercase) is for the opposite (i.e. non-whitespace).

[JeffryBooher]

I guess on an iPhone it's hard to tell the difference :)

[RaymondLim]

@redmunds I'll submit a pull request to clean up all the usage of trim() for this purpose. I prefer to use string.match(/\S/) instead of string.search(/\S/) since we don't really need to know the position. See this http://jsperf.com/exec-vs-match-vs-test-vs-search/5 for a comparison.

[redmunds]

@RaymondLim According to that chart, it looks like /\S/.test(string) is even faster.

[RaymondLim]

You're right! I was interpreting the opposite.

[JeffryBooher]

@RaymondLim, I'm already working on a pull request