Add support for grid line names

[raxbg]

Fix proposal for #131

[sabberworm]

Use consumeWhitespace after [ and before ].

[raxbg]

Should be okay now. Please check.

[sabberworm]

Reading https://developer.mozilla.org/en-US/docs/Web/CSS/custom-ident it seems we should support certain escape sequences. We could use parseCharacter(true) for this.

[raxbg]

Should be okay now. Please check.

[sabberworm]

Lenient mode has always tried to mimic what the browser does when parsing. I think if the browser encounters an unparseable value, it throws away the whole rule so we should do the same. Since there is a handler for UnexpectedTokenException already in place that does that, we don’t need to catch it in lenient mode.

Then again I’m not sure what the other half of the if (!$sName || !$this->comes(']')) is for. The comment does not enlighten me much. Shouldn’t [ linename ] parse correctly without throwing an exception? There’s something I don’t understand.

[raxbg]

Lenient mode has always tried to mimic what the browser does when parsing. I think if the browser encounters an unparseable value, it throws away the whole rule so we should do the same.

[] is parsed as [] in Chrome. If the UnexpectedTokenException is not handled here the parser will drop the value for the rule.

Then again I’m not sure what the other half of the if (!$sName || !$this->comes(']')) is for.

On the second iteration of the loop, parseIdentifier() will try to parse the ], so it will throw an exception. However this should not be considered an error because $sName will not be empty (it will have the value of 'linename '). The exception should be forwarded only if $sName is empty and lenient mode is disabled. This will be the case if parsing [].

[sabberworm]

Ok, you have convinced me with the lenient mode exception handling.

[sabberworm]

I still think using exception handling for control flow should be avoided. Having read your comments, shouldn’t the following also work?:

private function parseLineNameValue() {
	$this->consume('[');
	$this->consumeWhiteSpace();
	$sName = '';
	do {
		if($this->oParserSettings->bLenientParsing) {
			try {
				$sName .= $this->parseIdentifier(false, true) . ' ';
			} catch(UnexpectedTokenException $e) {}
		} else {
			$sName .= $this->parseIdentifier(false, true) . ' ';
		}
		$this->consumeWhiteSpace();
	} while (!$this->comes(']'));
	$this->consume(']');
	return new LineName(rtrim($sName), $this->iLineNo);
}

Having said that: now that I know there can be multiple identifiers in line name value brackets, I think LineName should extend ValueList with $sSeparator = ' ' and thus the code would become:

private function parseLineNameValue() {
	$this->consume('[');
	$this->consumeWhiteSpace();
	$aNames = array();
	do {
		if($this->oParserSettings->bLenientParsing) {
			try {
				$aNames[] = $this->parseIdentifier(false, true);
			} catch(UnexpectedTokenException $e) {}
		} else {
			$aNames[] = $this->parseIdentifier(false, true);
		}
		$this->consumeWhiteSpace();
	} while (!$this->comes(']'));
	$this->consume(']');
	return new LineName($aNames, $this->iLineNo);
}

which would allow you to shorten LineName to the following:

class LineName extends ValueList {
	public function __construct($aComponents = array(), $iLineNo = 0) {
		parent::__construct($aComponents, ' ', $iLineNo);
	}

	public function __toString() {
		return $this->render(new \Sabberworm\CSS\OutputFormat());
	}

	public function render(\Sabberworm\CSS\OutputFormat $oOutputFormat) {
		return '['.parent::render(\Sabberworm\CSS\OutputFormat::createCompact()).']';
	}
}

Optionally, you could create OutputFormat options on whether or not white-space should be output after [ and before ].

[raxbg]

Love it ❤️

[sabberworm]

Thanks a lot @raxbg !