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

Add support for various features and fixes #583

Closed
wants to merge 1 commit into from

Conversation

niaccurshi
Copy link

references #310, #558, #556, #577 and at least some of #511, wide changes to primarily aid parent selectors and @at-root

Fixes to resolve issues such as using the parent selector in assigns, ability to use bracketed lists, the proper use for double ampersands (using interpolation), correct selectors for @at-root and better finding of variables from within nested mixins.
Copy link
Collaborator

@robocoder robocoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I cannot merge this PR as is.

@@ -316,16 +316,9 @@ protected function makeOutputBlock($type, $selectors = null)
$out->parent = $this->scope;
$out->selectors = $selectors;
$out->depth = $this->env->depth;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? It appears to revert the fix for #570

@@ -775,22 +768,50 @@ protected function compileDirective(Block $block)
*/
protected function compileAtRoot(Block $block)
{

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if you update your PR WRT PSR2.

In addition:

  • avoid extraneous blank lines,
  • space after foreach keyword

@@ -1620,12 +1724,40 @@ protected function compileChild($child, OutputBlock $out)

case Type::T_ASSIGN:
list(, $name, $value) = $child;
$stups = $value[0];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$stups appears to be unused

@@ -2701,9 +2838,7 @@ public function compileValue($value)
$b = round($b);

if (count($value) === 5 && $value[4] !== 1) { // rgba
$a = new Node\Number($value[4], '');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? It appears to revert the fix for #573

@@ -3714,7 +3879,7 @@ protected function sortArgs($prototype, $args)
$key = $key[1];

if (empty($key)) {
$posArgs[] = empty($arg[2]) ? $value : $arg;
Copy link
Collaborator

@robocoder robocoder Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? It appears to revert the fix for #569

Copy link
Collaborator

@robocoder robocoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the feeling that you overwrite the file from an out-of-date branch...

@@ -4358,7 +4505,7 @@ protected function libRgb($args)
protected function libRgba($args)
{
if ($color = $this->coerceColor($args[0])) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto #573

@niaccurshi
Copy link
Author

Hey, it appears that somehow I have got a branch out of date here. Let me go back and review what's happened and merge my changes with the most up to date version, I'll make a new set of pull requests.

@niaccurshi niaccurshi closed this Jul 19, 2018
@groovenectar
Copy link

groovenectar commented Aug 13, 2018

@niaccurshi Were you able to fix at-root issues? I think it is still present in 0.7.7

@groovenectar
Copy link

It seems that I still see this odd error using the update:
/* Undefined variable $min: line: -1 */
As outlined here: #579

@niaccurshi
Copy link
Author

I don't know that what I was altering had any bearing on that particular problem. My changes fix a range of issues that are present, but certainly not all.

@niaccurshi niaccurshi reopened this Aug 13, 2018
@niaccurshi niaccurshi closed this Aug 13, 2018
@niaccurshi
Copy link
Author

Sorry, I realise I'm in the wrong thread here, see #593 for the updated changes.

@niaccurshi niaccurshi deleted the patch-1 branch August 13, 2018 23:28
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.

None yet

3 participants