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

Condition (helper) inside can-EVENT doesn't work #1800

Closed
whitecolor opened this Issue Jul 22, 2015 · 42 comments

Comments

Projects
None yet
6 participants
@whitecolor
Contributor

whitecolor commented Jul 22, 2015

in latest minor pre 2.3 code:

<div can-click="{{#if canShowPopover}}togglePopover{{/if}}"></div>

causes an error:

Uncaught TypeError: Cannot read property 'compile' of undefined .../node_modules/can/view/stache/text_section.js 29 TypeError: Cannot read property 'compile' of undefined

it works in 2.2.6

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jul 22, 2015

Contributor

I don't think this is valid. We can fix it, but the #if should go on the outside of the whole attribute.

Sent from my iPhone

On Jul 22, 2015, at 5:12 PM, Alex notifications@github.com wrote:

in latest minor pre 2.3 code:

causes an error:

Uncaught TypeError: Cannot read property 'compile' of undefined .../node_modules/can/view/stache/text_section.js 29 TypeError: Cannot read property 'compile' of undefined
it works in 2.2.6


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Jul 22, 2015

I don't think this is valid. We can fix it, but the #if should go on the outside of the whole attribute.

Sent from my iPhone

On Jul 22, 2015, at 5:12 PM, Alex notifications@github.com wrote:

in latest minor pre 2.3 code:

causes an error:

Uncaught TypeError: Cannot read property 'compile' of undefined .../node_modules/can/view/stache/text_section.js 29 TypeError: Cannot read property 'compile' of undefined
it works in 2.2.6


Reply to this email directly or view it on GitHub.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Jul 22, 2015

Contributor

Why not valid? I find it useful often:

<div can-click="{{#if shown}}hide{{else}}show{{/if}}"></div>

or just helper action (that contains string name of calling method):

<div can-click="{{currentAction}}"></div>
Contributor

whitecolor commented Jul 22, 2015

Why not valid? I find it useful often:

<div can-click="{{#if shown}}hide{{else}}show{{/if}}"></div>

or just helper action (that contains string name of calling method):

<div can-click="{{currentAction}}"></div>
@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Jul 22, 2015

Contributor

I think this may be related #1770

Contributor

whitecolor commented Jul 22, 2015

I think this may be related #1770

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Jul 27, 2015

Contributor

@daffl is it difficult to fix in minor?

Contributor

whitecolor commented Jul 27, 2015

@daffl is it difficult to fix in minor?

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jul 29, 2015

Contributor

Doesn't a helper just get used like

<div can-click="{{#canShowPopover}}togglePopover{{/canShowPopover}}"></div>

Also your example does not have a closing {{/if}}.

Contributor

daffl commented Jul 29, 2015

Doesn't a helper just get used like

<div can-click="{{#canShowPopover}}togglePopover{{/canShowPopover}}"></div>

Also your example does not have a closing {{/if}}.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Jul 29, 2015

Contributor

@daffl it just a mistype, doesn't work any way

Contributor

whitecolor commented Jul 29, 2015

@daffl it just a mistype, doesn't work any way

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Aug 19, 2015

Contributor

Are you going to fix this soon?

Contributor

whitecolor commented Aug 19, 2015

Are you going to fix this soon?

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Aug 19, 2015

Contributor

Probably not soon, PRs welcome though.

Contributor

matthewp commented Aug 19, 2015

Probably not soon, PRs welcome though.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Aug 20, 2015

Contributor

@matthewp not sure where to look it.

Contributor

whitecolor commented Aug 20, 2015

@matthewp not sure where to look it.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Aug 20, 2015

Contributor

I'll have a look next week. If this worked in 2.2 we should probably find out what changed.

Contributor

daffl commented Aug 20, 2015

I'll have a look next week. If this worked in 2.2 we should probably find out what changed.

@daffl daffl added this to the 2.3.0 milestone Aug 20, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Aug 20, 2015

Contributor

We turned off processing in can.view.attrs to make it so we can switch between single binding and two way binding.

Sent from my iPhone

On Aug 20, 2015, at 7:13 AM, David Luecke notifications@github.com wrote:

I'll have a look next week. If this worked in 2.2 we should probably find out what changed.


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Aug 20, 2015

We turned off processing in can.view.attrs to make it so we can switch between single binding and two way binding.

Sent from my iPhone

On Aug 20, 2015, at 7:13 AM, David Luecke notifications@github.com wrote:

I'll have a look next week. If this worked in 2.2 we should probably find out what changed.


Reply to this email directly or view it on GitHub.

@beno

This comment has been minimized.

Show comment
Hide comment
@beno

beno Aug 26, 2015

Contributor

Never mind the conditionals, it looks like mustache has been completely disabled in attributes? My navigation component is now broken (I use SLIM and the ~ is short for {{ }})

nav
 ul.nav.clearfix
   ~#actions
     li
       a href="#" can-click="~." ~.

To avoid confusion, here's what this gets translated into:

<nav>
  <ul class="nav clearfix">
    {{#actions}}
      <li><a can-click="{{.}}" href="#">{{.}}</a></li>
    {{/actions}}
  </ul>
</nav>

This all works fine in 2.2.7, but fails in 2.3

scopeData.value.apply is not a function
Contributor

beno commented Aug 26, 2015

Never mind the conditionals, it looks like mustache has been completely disabled in attributes? My navigation component is now broken (I use SLIM and the ~ is short for {{ }})

nav
 ul.nav.clearfix
   ~#actions
     li
       a href="#" can-click="~." ~.

To avoid confusion, here's what this gets translated into:

<nav>
  <ul class="nav clearfix">
    {{#actions}}
      <li><a can-click="{{.}}" href="#">{{.}}</a></li>
    {{/actions}}
  </ul>
</nav>

This all works fine in 2.2.7, but fails in 2.3

scopeData.value.apply is not a function
@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Aug 26, 2015

Contributor

@beno This only applies to custom attributes defined with can.view.attr which I've brought up in basically a duplicate issue here.

What basically happens now is the can.view.attr callback just receives a string before processing. This is necessary to make the new references bindings work because the #-prefixed attributes need to see the raw string to determine whether to hook up a 1-way or 2-way binding. This probably would have been avoided if the references bindings were part of can.Component instead of a custom attribute pattern.

The probable resolution would be to modify existing custom attribute definitions (particularly can-EVENT) to do the processing internally.

Contributor

dylanrtt commented Aug 26, 2015

@beno This only applies to custom attributes defined with can.view.attr which I've brought up in basically a duplicate issue here.

What basically happens now is the can.view.attr callback just receives a string before processing. This is necessary to make the new references bindings work because the #-prefixed attributes need to see the raw string to determine whether to hook up a 1-way or 2-way binding. This probably would have been avoided if the references bindings were part of can.Component instead of a custom attribute pattern.

The probable resolution would be to modify existing custom attribute definitions (particularly can-EVENT) to do the processing internally.

@beno

This comment has been minimized.

Show comment
Hide comment
@beno

beno Aug 26, 2015

Contributor

Not sure I follow. Are you saying the behavior of the attributes (like can-EVENT) has been changed to accommodate #-prefixed attributes? And is this considered a feature or a bug? If feature, how would one create the logic in my example from 2.3 onward?

To be sure, the 'actions'-array in my code is a simple list of strings with each string having a corresponding function in the viewModel. So I am not doing any custom attributes or anything like that.

Contributor

beno commented Aug 26, 2015

Not sure I follow. Are you saying the behavior of the attributes (like can-EVENT) has been changed to accommodate #-prefixed attributes? And is this considered a feature or a bug? If feature, how would one create the logic in my example from 2.3 onward?

To be sure, the 'actions'-array in my code is a simple list of strings with each string having a corresponding function in the viewModel. So I am not doing any custom attributes or anything like that.

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Aug 26, 2015

Contributor

The behavior of registering a custom attribute callback with can.view.attr was changed so the #-prefixed attributes which are registered with can.view.attr (/#[\w\.\-_]+/ pattern) can get the raw string instead of a processed one. This was a necessary feature to make it work and now people are seeing the backwards-incompatible side effect.

I was referring to what could be done to resolve it within the framework. Since can-EVENT is registered with can.view.attr, that's why this is a problem, and the function defined for can-EVENT could be updated to process the string.

Justin also threw out the idea of making a special syntax for enforcing that the mustache code is processed before the attribute callback receives it, but I'm not sure I like that.

Unfortunately, I don't think there's anything you can do within the context of your app to fix this without adding unnecessary complexity.

Contributor

dylanrtt commented Aug 26, 2015

The behavior of registering a custom attribute callback with can.view.attr was changed so the #-prefixed attributes which are registered with can.view.attr (/#[\w\.\-_]+/ pattern) can get the raw string instead of a processed one. This was a necessary feature to make it work and now people are seeing the backwards-incompatible side effect.

I was referring to what could be done to resolve it within the framework. Since can-EVENT is registered with can.view.attr, that's why this is a problem, and the function defined for can-EVENT could be updated to process the string.

Justin also threw out the idea of making a special syntax for enforcing that the mustache code is processed before the attribute callback receives it, but I'm not sure I like that.

Unfortunately, I don't think there's anything you can do within the context of your app to fix this without adding unnecessary complexity.

@beno

This comment has been minimized.

Show comment
Hide comment
@beno

beno Aug 26, 2015

Contributor

OK, thanks for the explanation, I get it now.

Unfortunately, I don't think there's anything you can do within the context of your app to fix this without adding unnecessary complexity.

So there is no way to have any kind of dynamic custom attribute? That is quite the regression, don't think I have ever encountered something like this before. Besides, doesn't a 1-way vs 2-way binding only make sense in can-value attributes?

Contributor

beno commented Aug 26, 2015

OK, thanks for the explanation, I get it now.

Unfortunately, I don't think there's anything you can do within the context of your app to fix this without adding unnecessary complexity.

So there is no way to have any kind of dynamic custom attribute? That is quite the regression, don't think I have ever encountered something like this before. Besides, doesn't a 1-way vs 2-way binding only make sense in can-value attributes?

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Aug 26, 2015

Contributor

Hold on, first this is a legitimate bug as it is a regression, no one is saying this is a "won't fix". It's probably a hard thing to fix though, would you be ok if it was "opt in" so that you enabled it some how? That's my first thought.

It's probably possible to do what you want to do by changing can-click to read . from the scope. This can definitely be fixed.

Contributor

matthewp commented Aug 26, 2015

Hold on, first this is a legitimate bug as it is a regression, no one is saying this is a "won't fix". It's probably a hard thing to fix though, would you be ok if it was "opt in" so that you enabled it some how? That's my first thought.

It's probably possible to do what you want to do by changing can-click to read . from the scope. This can definitely be fixed.

@beno

This comment has been minimized.

Show comment
Hide comment
@beno

beno Aug 26, 2015

Contributor

Ah, it was unclear to me whether it was considered a bug or not. I think I'd rather have a special way to enable the new behavior and leave the existing functionality as is. But flipping this around would be the next best thing I suppose.

Not sure what you meant by your last remark. Can you elaborate? Thanks.

Contributor

beno commented Aug 26, 2015

Ah, it was unclear to me whether it was considered a bug or not. I think I'd rather have a special way to enable the new behavior and leave the existing functionality as is. But flipping this around would be the next best thing I suppose.

Not sure what you meant by your last remark. Can you elaborate? Thanks.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Aug 26, 2015

Contributor

@beno before we discuss the fix it would be really awesome to get a minimal breaking test. If you could submit a PR with a breaking test here we can discuss a fix.

Contributor

matthewp commented Aug 26, 2015

@beno before we discuss the fix it would be really awesome to get a minimal breaking test. If you could submit a PR with a breaking test here we can discuss a fix.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Aug 26, 2015

Contributor

There's 2 things at play here; fixing can-EVENT probably pretty easy, but fixing it for all can.view.attrs would not be.

Contributor

matthewp commented Aug 26, 2015

There's 2 things at play here; fixing can-EVENT probably pretty easy, but fixing it for all can.view.attrs would not be.

@matthewp matthewp added the bug label Aug 26, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Aug 26, 2015

Contributor

To be clear, a dynamic custom attribute was never intended to work. Just a happy accident. So it's a gray area if this is a bug or regression.

However, we can still make it work by allowing can-value to make the attribute a template, running the template, and seeing the result.

Sent from my iPhone

On Aug 26, 2015, at 1:45 PM, beno notifications@github.com wrote:

OK, thanks for the explanation, I get it now.

Unfortunately, I don't think there's anything you can do within the context of your app to fix this without adding unnecessary complexity.

So there is no way to have any kind of dynamic custom attribute? That is quite the regression, don't think I have ever encountered something like this before. Besides, doesn't a 1-way vs 2-way binding only make sense in can-value attributes?


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Aug 26, 2015

To be clear, a dynamic custom attribute was never intended to work. Just a happy accident. So it's a gray area if this is a bug or regression.

However, we can still make it work by allowing can-value to make the attribute a template, running the template, and seeing the result.

Sent from my iPhone

On Aug 26, 2015, at 1:45 PM, beno notifications@github.com wrote:

OK, thanks for the explanation, I get it now.

Unfortunately, I don't think there's anything you can do within the context of your app to fix this without adding unnecessary complexity.

So there is no way to have any kind of dynamic custom attribute? That is quite the regression, don't think I have ever encountered something like this before. Besides, doesn't a 1-way vs 2-way binding only make sense in can-value attributes?


Reply to this email directly or view it on GitHub.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Aug 26, 2015

Contributor

I didn't want to make it part of component because other things can have viewModels. I want this to work for them as well.

Sent from my iPhone

On Aug 26, 2015, at 12:45 PM, dylanrtt notifications@github.com wrote:

@beno This only applies to custom attributes defined with can.view.attr which I've brought up in basically a duplicate issue here.

What basically happens now is the can.view.attr callback just receives a string before processing. This is necessary to make the new references bindings work because the #-prefixed attributes need to see the raw string to determine whether to hook up a 1-way or 2-way binding. This probably would have been avoided if the references bindings were part of can.Component instead of a custom attribute pattern.

The probable resolution would be to modify existing custom attribute callbacks, particularly can-EVENT, to do the processing internally.


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Aug 26, 2015

I didn't want to make it part of component because other things can have viewModels. I want this to work for them as well.

Sent from my iPhone

On Aug 26, 2015, at 12:45 PM, dylanrtt notifications@github.com wrote:

@beno This only applies to custom attributes defined with can.view.attr which I've brought up in basically a duplicate issue here.

What basically happens now is the can.view.attr callback just receives a string before processing. This is necessary to make the new references bindings work because the #-prefixed attributes need to see the raw string to determine whether to hook up a 1-way or 2-way binding. This probably would have been avoided if the references bindings were part of can.Component instead of a custom attribute pattern.

The probable resolution would be to modify existing custom attribute callbacks, particularly can-EVENT, to do the processing internally.


Reply to this email directly or view it on GitHub.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Aug 26, 2015

Contributor

@justinbmeyer can't we just check for {{foo}} and then lookup foo in the scope?

Contributor

matthewp commented Aug 26, 2015

@justinbmeyer can't we just check for {{foo}} and then lookup foo in the scope?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Aug 26, 2015

Contributor

Yes, but I think we could make "{{#mobile}}touch{{else}}click{{/}}" work too.

Sent from my iPhone

On Aug 26, 2015, at 3:16 PM, Matthew Phillips notifications@github.com wrote:

@justinbmeyer can't we just check for {{foo}} and then lookup foo in the scope?


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Aug 26, 2015

Yes, but I think we could make "{{#mobile}}touch{{else}}click{{/}}" work too.

Sent from my iPhone

On Aug 26, 2015, at 3:16 PM, Matthew Phillips notifications@github.com wrote:

@justinbmeyer can't we just check for {{foo}} and then lookup foo in the scope?


Reply to this email directly or view it on GitHub.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Aug 26, 2015

Contributor

"{{#mobile}}touch{{else}}click{{/}}" I use such often

or such pattern:

can-click="{{clickMethod}}" where clickMethod: function(){this.attr('mobile') ? 'touch' : 'click'}

Contributor

whitecolor commented Aug 26, 2015

"{{#mobile}}touch{{else}}click{{/}}" I use such often

or such pattern:

can-click="{{clickMethod}}" where clickMethod: function(){this.attr('mobile') ? 'touch' : 'click'}

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Oct 9, 2015

Contributor

Will it be fixed soon? The only thing that holds to migrate one of our projects to 2.3beta.

Contributor

whitecolor commented Oct 9, 2015

Will it be fixed soon? The only thing that holds to migrate one of our projects to 2.3beta.

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Oct 9, 2015

Contributor

@whitecolor The new binding syntax should allow this. I haven't tried it yet but you might be able to upgrade now.

<input type="button" ($click)="{{#if shown}}hide{{else}}show{{/if}}">

I believe can-click will also work since processing should be turned back on for custom attributes now that the references scope bindings have different syntax.

Contributor

dylanrtt commented Oct 9, 2015

@whitecolor The new binding syntax should allow this. I haven't tried it yet but you might be able to upgrade now.

<input type="button" ($click)="{{#if shown}}hide{{else}}show{{/if}}">

I believe can-click will also work since processing should be turned back on for custom attributes now that the references scope bindings have different syntax.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Oct 9, 2015

Contributor

Yes, please try with 2.3-beta and let us know.

Contributor

matthewp commented Oct 9, 2015

Yes, please try with 2.3-beta and let us know.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 9, 2015

Contributor

@whitecolor I don't think 2.3 supports this yet. I never changed the behavior of custom attributes to not be processed. However, reverting this change should be possible now. Please revert it and let us know. It would be good to get this in 2.3 while there's still time. Thanks!

Contributor

justinbmeyer commented Oct 9, 2015

@whitecolor I don't think 2.3 supports this yet. I never changed the behavior of custom attributes to not be processed. However, reverting this change should be possible now. Please revert it and let us know. It would be good to get this in 2.3 while there's still time. Thanks!

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Oct 10, 2015

Contributor

@dylanrtt @matthewp @justinbmeyer
I've checked on beta4 it doesn't work, error stack:

Potentially unhandled rejection [14] TypeError: Cannot read property 'compile' of undefined
    at can.extend.compile (.../node_modules/can/view/stache/text_section.js:29:32)
    at Object.parser.attrEnd (.../node_modules/can/view/stache/stache.js:216:47)
    at callAttrEnd (.../node_modules/can/view/parser/parser.js:285:11)
    at Function.HTMLParser.parseAttrs (.../node_modules/can/view/parser/parser.js:348:7)
    at parseStartTag (.../node_modules/can/view/parser/parser.js:112:15)
    at String.replace (native)
    at HTMLParser (.../node_modules/can/view/parser/parser.js:214:16)
    at stache (.../node_modules/can/view/stache/stache.js:122:3)
    at Object.can.view.register.fragRenderer (.../node_modules/can/view/stache/stache.js:349:11)
    at Function.can.extend.register.can.(anonymous function).$view.(anonymous function).renderFunc [as renderer] (.../node_modules/can/view/view.js:404:25)
Contributor

whitecolor commented Oct 10, 2015

@dylanrtt @matthewp @justinbmeyer
I've checked on beta4 it doesn't work, error stack:

Potentially unhandled rejection [14] TypeError: Cannot read property 'compile' of undefined
    at can.extend.compile (.../node_modules/can/view/stache/text_section.js:29:32)
    at Object.parser.attrEnd (.../node_modules/can/view/stache/stache.js:216:47)
    at callAttrEnd (.../node_modules/can/view/parser/parser.js:285:11)
    at Function.HTMLParser.parseAttrs (.../node_modules/can/view/parser/parser.js:348:7)
    at parseStartTag (.../node_modules/can/view/parser/parser.js:112:15)
    at String.replace (native)
    at HTMLParser (.../node_modules/can/view/parser/parser.js:214:16)
    at stache (.../node_modules/can/view/stache/stache.js:122:3)
    at Object.can.view.register.fragRenderer (.../node_modules/can/view/stache/stache.js:349:11)
    at Function.can.extend.register.can.(anonymous function).$view.(anonymous function).renderFunc [as renderer] (.../node_modules/can/view/view.js:404:25)

@daffl daffl modified the milestone: 2.3.0 Oct 22, 2015

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Oct 27, 2015

Contributor

What about this?

Contributor

whitecolor commented Oct 27, 2015

What about this?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 27, 2015

Contributor

Did you try reverting? Does it work?

Sent from my iPhone

On Oct 27, 2015, at 1:27 PM, Alex notifications@github.com wrote:

What about this?


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Oct 27, 2015

Did you try reverting? Does it work?

Sent from my iPhone

On Oct 27, 2015, at 1:27 PM, Alex notifications@github.com wrote:

What about this?


Reply to this email directly or view it on GitHub.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Oct 27, 2015

Contributor

Did you try reverting? Does it work?

What do you mean?

Contributor

whitecolor commented Oct 27, 2015

Did you try reverting? Does it work?

What do you mean?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 27, 2015

Contributor

A few comments previous, I mentioned trying to revert the change that custom attrs are not processed.

Sent from my iPhone

On Oct 27, 2015, at 2:30 PM, Alex notifications@github.com wrote:

Did you try reverting? Does it work?

What do you mean?


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Oct 27, 2015

A few comments previous, I mentioned trying to revert the change that custom attrs are not processed.

Sent from my iPhone

On Oct 27, 2015, at 2:30 PM, Alex notifications@github.com wrote:

Did you try reverting? Does it work?

What do you mean?


Reply to this email directly or view it on GitHub.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Oct 27, 2015

Contributor

the same result with an error, is the test needed?

Contributor

whitecolor commented Oct 27, 2015

the same result with an error, is the test needed?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 27, 2015

Contributor

a test will speed up the process of getting this fixed. What was the error?

Contributor

justinbmeyer commented Oct 27, 2015

a test will speed up the process of getting this fixed. What was the error?

@beno

This comment has been minimized.

Show comment
Hide comment
@beno

beno Oct 27, 2015

Contributor

I made this test a while ago

#1863

Contributor

beno commented Oct 27, 2015

I made this test a while ago

#1863

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 27, 2015

Contributor

@beno thanks!

Contributor

justinbmeyer commented Oct 27, 2015

@beno thanks!

@daffl daffl modified the milestones: 2.3.1, 2.3.2 Oct 28, 2015

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Nov 5, 2015

Contributor

When is it going to be fixed? Holding from upgrade.

Contributor

whitecolor commented Nov 5, 2015

When is it going to be fixed? Holding from upgrade.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 6, 2015

Contributor

@whitecolor what was the error you saw when you tried the above? This isn't a super high priority fix for us. We have it slated for 2.3.2. But there's 20 or so other issues in this release. Any help you can give will be greatly appreciated.

Contributor

justinbmeyer commented Nov 6, 2015

@whitecolor what was the error you saw when you tried the above? This isn't a super high priority fix for us. We have it slated for 2.3.2. But there's 20 or so other issues in this release. Any help you can give will be greatly appreciated.

@justinbmeyer justinbmeyer self-assigned this Nov 9, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 9, 2015

Contributor

@whitecolor I changed the code, got no errors, and all tests passed, including the one submitted by @beno . Can you confirm that you actually got an error?

Contributor

justinbmeyer commented Nov 9, 2015

@whitecolor I changed the code, got no errors, and all tests passed, including the one submitted by @beno . Can you confirm that you actually got an error?

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Nov 9, 2015

Contributor

Closed via #2066

Contributor

daffl commented Nov 9, 2015

Closed via #2066

@daffl daffl closed this Nov 9, 2015

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