fix(parser): allow whitespace between function name and argument list - #943
Draft
npazosmendez wants to merge 4 commits into
Draft
fix(parser): allow whitespace between function name and argument list#943npazosmendez wants to merge 4 commits into
npazosmendez wants to merge 4 commits into
Conversation
parseExprWithoutPipe decided a name was a function call by testing whether
the next byte was '(', so `exclude ('foo')` parsed as the metric name
`exclude`. Piped, that failed with "pipe to not a function"; at top level
the argument list was silently dropped.
graphite-web's grammar builds a call as `funcname + leftParen` in pyparsing,
which skips intervening whitespace, so it accepts both spellings.
Whitespace is consumed only when a '(' follows, so the non-call path returns
its remainder unchanged.
npazosmendez
force-pushed
the
njpm/parser-space-before-paren
branch
from
August 4, 2026 19:38
bba834c to
34f6deb
Compare
…nction calls The lookahead applied to every parsed name, which broke callers that re-parse series names and discard the remainder (aliasQuery, applyByNode): a name like `metric1 (avg: 3)` became a hard parse error. It also turned metric paths such as `servers.web01.cpu (percent)` into unknown-function errors, and let groupByNode callbacks like "sum (x)" slip past the EtFunc guard, silently dropping every group. The whitespace is now consumed only when the name matches graphite-web's funcname grammar (Word(alphas+'_', alphanums+'_')) and the argument list parses; otherwise the name is returned with the remainder untouched, as before. groupByNode additionally rejects callbacks whose parse leaves a remainder.
The whitespace lookahead parsed the argument list speculatively and then re-parsed it in the call branch. Extract the call construction into parseCall and return its result directly, so each argument list is parsed once. No behavior change.
npazosmendez
force-pushed
the
njpm/parser-space-before-paren
branch
from
August 5, 2026 12:08
07ad783 to
c9ba25b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
function ('foo')does not parse. Whether a name starts a function call was decided by testing the byte immediately after it for(, so a name followed by whitespace parsed as a metric name instead.graphite-web accepts both spellings. Its
callisfuncname + leftParen, and pyparsing skips whitespace before each element of anAnd:https://github.com/graphite-project/graphite-web/blob/1.1.10/webapp/graphite/render/grammar_unsafe.py#L86-L95
The whitespace is consumed only when
(follows, so the non-call path returns its remainder unchanged.See graphite web test showing a succesfull parse: