Skip to content

Add missing parent call for Table::initialize().#124

Merged
AD7six merged 4 commits intomasterfrom
master-table
Jul 19, 2015
Merged

Add missing parent call for Table::initialize().#124
AD7six merged 4 commits intomasterfrom
master-table

Conversation

@dereuromark
Copy link
Copy Markdown
Member

Closes #120

@dereuromark dereuromark added this to the 1.0.11 milestone Jun 19, 2015
@dereuromark
Copy link
Copy Markdown
Member Author

Is there a way to remove that whitespace noise in the bake templates when generating?
See failing tests.
IMO those should be trimmed right.

@AD7six
Copy link
Copy Markdown
Member

AD7six commented Jun 19, 2015

It fails because bakes output contains extra whitespace - the correct fix is to fix the output, looks to me like this is responsible:

<%= "            " . implode("\n            ", $validationMethods) %>

There shouldn't be any "echo whitespace" logic in these templates, only echo statements at the right indentation level.

@dereuromark
Copy link
Copy Markdown
Member Author

Or running a trim right on the generated code on a per row level.
Either way, the generated code shouldn't contain whitespace noise.

@AD7six
Copy link
Copy Markdown
Member

AD7six commented Jun 19, 2015

Or running a trim right on the generated code on a per row level.

I don't want that kind of logic - the template is right, or it's not. not mostly-right with magic applied. There'll be cases where you want/expect trailing whitespace and that'd prevent it.

@markstory
Copy link
Copy Markdown
Member

Seems fine one the tests pass.

@dereuromark
Copy link
Copy Markdown
Member Author

Feel free to fix the unrelated whitespace issue either as commits on top or separate PR.
Then this one will be fine to merge.

@AD7six
Copy link
Copy Markdown
Member

AD7six commented Jun 19, 2015

@dereuromark it's changes in this pr that cause the test fails (it previously matched output, unrelated changes to the test file in this PR make it fail).

As I've mentioned before its easy to get the test output to match expectations - I can fix the now-apparent bad template later/tomorrow.

AD7six added 2 commits July 19, 2015 14:29
This shouldn't be in any templates
Whitespace should never be output by php code. Also, implode shouldn't
be abused to construct code like that either
AD7six added a commit that referenced this pull request Jul 19, 2015
Add missing parent call for Table::initialize().
@AD7six AD7six merged commit 3f007f7 into master Jul 19, 2015
@AD7six AD7six deleted the master-table branch July 19, 2015 14:45
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