Digestor explicit dependency should not contain trailing whitespace #8586

Merged
merged 1 commit into from Dec 22, 2012

Conversation

Projects
None yet
3 participants
@balexand
Contributor

balexand commented Dec 21, 2012

This is an extremely simple and low impact change in the regex used to find explicit template dependencies. This allows Digestor to be used with HAML. Please see rails/cache_digests#21 for details.

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Dec 21, 2012

Contributor

We probably need tests.

Contributor

josevalim commented Dec 21, 2012

We probably need tests.

@balexand

This comment has been minimized.

Show comment Hide comment
@balexand

balexand Dec 21, 2012

Contributor

@josevalim I'd be happy to include tests. Would you prefer that I create a single HAML fixture that reproduces this bug? Or would you prefer that I port the entire test/fixtures/digestor folder to include HAML in addition to ERB like I did in rails/cache_digests#22?

Contributor

balexand commented Dec 21, 2012

@josevalim I'd be happy to include tests. Would you prefer that I create a single HAML fixture that reproduces this bug? Or would you prefer that I port the entire test/fixtures/digestor folder to include HAML in addition to ERB like I did in rails/cache_digests#22?

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Dec 21, 2012

Member

Do we really need to use HAML. Can't we use a ERB template with the same string generated from HAML?

Member

rafaelfranca commented Dec 21, 2012

Do we really need to use HAML. Can't we use a ERB template with the same string generated from HAML?

@balexand

This comment has been minimized.

Show comment Hide comment
@balexand

balexand Dec 22, 2012

Contributor

@rafaelfranca Sure, how about this?

https://github.com/balexand/rails/commit/13418563fac197256bc93c15f6905505391df373

If you like it then I'll merge it into this pull-request. I'm open to feedback. Thanks!

Contributor

balexand commented Dec 22, 2012

@rafaelfranca Sure, how about this?

https://github.com/balexand/rails/commit/13418563fac197256bc93c15f6905505391df373

If you like it then I'll merge it into this pull-request. I'm open to feedback. Thanks!

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Dec 22, 2012

Member

Seems good

Member

rafaelfranca commented Dec 22, 2012

Seems good

@balexand

This comment has been minimized.

Show comment Hide comment
@balexand

balexand Dec 22, 2012

Contributor

Cool, I merged the test into this pull-request.

Contributor

balexand commented Dec 22, 2012

Cool, I merged the test into this pull-request.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Dec 22, 2012

Member

Could you squash your commits?

Member

rafaelfranca commented Dec 22, 2012

Could you squash your commits?

@balexand

This comment has been minimized.

Show comment Hide comment
@balexand

balexand Dec 22, 2012

Contributor

Sure, commits have been squashed.

Contributor

balexand commented Dec 22, 2012

Sure, commits have been squashed.

rafaelfranca added a commit that referenced this pull request Dec 22, 2012

Merge pull request #8586 from balexand/cache_digests_regex
Digestor explicit dependency should not contain trailing whitespace

@rafaelfranca rafaelfranca merged commit 8a39d83 into rails:master Dec 22, 2012

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Dec 22, 2012

Member

Thank you

Member

rafaelfranca commented Dec 22, 2012

Thank you

@balexand

This comment has been minimized.

Show comment Hide comment
@balexand

balexand Dec 22, 2012

Contributor

Thanks!

Contributor

balexand commented Dec 22, 2012

Thanks!

@elia elia referenced this pull request in rails/cache_digests Feb 13, 2013

Closed

Match non-whitespace characters instead of non-space characters #21

@balexand balexand referenced this pull request in rails/cache_digests Feb 13, 2013

Closed

HAML tests #22

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