Skip to content
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

Allow to have no macros in Compiler #177

Merged
merged 1 commit into from May 16, 2018
Merged

Allow to have no macros in Compiler #177

merged 1 commit into from May 16, 2018

Conversation

@f3l1x
Copy link
Member

f3l1x commented May 16, 2018

  • bug fix: #176
  • BC break? no
  • doc PR: no needed

This PR has 2 changes:

  • upgrade nette/tester to v2.0
  • allow to use own Compiler with no defined macros
@f3l1x

This comment has been minimized.

Copy link
Member Author

f3l1x commented May 16, 2018

Ou shit, nette/tester v2.0 requires >=5.6. My mistake.

@@ -20,7 +20,7 @@
"ext-tokenizer": "*"
},
"require-dev": {
"nette/tester": "~1.7",
"nette/tester": "^2.0.1",

This comment has been minimized.

Copy link
@dg

dg May 16, 2018

Member

Please leave 1.7, it must work with PHP 5.4

This comment has been minimized.

Copy link
@f3l1x

f3l1x May 16, 2018

Author Member

Yop, my mistake.

@@ -124,7 +124,7 @@ public function compile(array $tokens, $className)
$this->methods = ['main' => null, 'prepare' => null];

$macroHandlers = new \SplObjectStorage;
array_map([$macroHandlers, 'attach'], call_user_func_array('array_merge', $this->macros));
array_map([$macroHandlers, 'attach'], $this->macros ? call_user_func_array('array_merge', $this->macros): []);

This comment has been minimized.

Copy link
@dg

dg May 16, 2018

Member

Why is it needed?

This comment has been minimized.

Copy link
@f3l1x

f3l1x May 16, 2018

Author Member
Exited with error code 255 (expected 0)
   E_WARNING: array_merge() expects at least 1 parameter, 0 given
@f3l1x f3l1x force-pushed the f3l1x:feature/no-macros branch from e8475ee to 85fd271 May 16, 2018
@@ -124,7 +124,10 @@ public function compile(array $tokens, $className)
$this->methods = ['main' => null, 'prepare' => null];

$macroHandlers = new \SplObjectStorage;
array_map([$macroHandlers, 'attach'], call_user_func_array('array_merge', $this->macros));

if ($this->macros) {

This comment has been minimized.

Copy link
@dg

dg May 16, 2018

Member

Is condition needed?

This comment has been minimized.

Copy link
@f3l1x

f3l1x May 16, 2018

Author Member

Yep, it triggers

Exited with error code 255 (expected 0)
   E_WARNING: array_merge() expects at least 1 parameter, 0 given

This comment has been minimized.

Copy link
@dg

dg May 16, 2018

Member

Ok, good.

@dg

This comment has been minimized.

Copy link
Member

dg commented May 16, 2018

Please fix coding style errors and I'll merge it.

@f3l1x f3l1x force-pushed the f3l1x:feature/no-macros branch from 85fd271 to 631fff5 May 16, 2018
@f3l1x

This comment has been minimized.

Copy link
Member Author

f3l1x commented May 16, 2018

Thanks.

@f3l1x f3l1x force-pushed the f3l1x:feature/no-macros branch from 631fff5 to 8ac6a71 May 16, 2018
@dg dg merged commit 6596b79 into nette:v2.4 May 16, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 93.566%
Details
dg added a commit that referenced this pull request May 16, 2018
dg added a commit that referenced this pull request May 17, 2018
dg added a commit that referenced this pull request Jun 3, 2018
dg added a commit that referenced this pull request Oct 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.