| Message ID | 20220607075425.39510-1-sluongng@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fsmonitor: query watchman with right valid json | expand |
On Tue, Jun 07 2022, Son Luong Ngoc wrote: > In rare circumstances where the current git index does not carry the > last_update_token, the fsmonitor v2 hook will be invoked with an > empty string which would caused the final rendered json to be invalid. > > ["query", "/path/to/my/git/repository/", { > "since": , > "fields": ["name"], > "expression": ["not", ["dirname", ".git"]] > }] > > Which will left user with the following error message > > > git status > failed to parse command from stdin: line 2, column 13, position 67: unexpected token near ',' > Watchman: command returned no output. > Falling back to scanning... > > Hide the "since" field in json query when "last_update_token" is empty. > > Signed-off-by: Son Luong Ngoc <sluongng@gmail.com> > --- > templates/hooks--fsmonitor-watchman.sample | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample > index 14ed0aa42d..b4ee86dfc4 100755 > --- a/templates/hooks--fsmonitor-watchman.sample > +++ b/templates/hooks--fsmonitor-watchman.sample > @@ -79,6 +79,12 @@ sub watchman_query { > or die "open2() failed: $!\n" . > "Falling back to scanning...\n"; > > + my $query = <<" END"; > + ["query", "$git_work_tree", { > + "fields": ["name"], > + "expression": ["not", ["dirname", ".git"]] > + }] > + END Wouldn't a more minimal & obvious patch here be.... > # In the query expression below we're asking for names of files that > # changed since $last_update_token but not from the .git folder. > # > @@ -87,15 +93,14 @@ sub watchman_query { > # output to file names only. Then we're using the "expression" term to > # further constrain the results. > if (substr($last_update_token, 0, 1) eq "c") { > - $last_update_token = "\"$last_update_token\""; To just change this to be: # same as now: $last_update_token = "\"$last_update_token\""; $last_update_line = qq["since": $last_update_token,]; Of course having declared the new $last_update_line variable earlier, then: > + $query = <<" END"; > + ["query", "$git_work_tree", { > + "since": "$last_update_token", > + "fields": ["name"], > + "expression": ["not", ["dirname", ".git"]] > + }] > + END > } > - my $query = <<" END"; > - ["query", "$git_work_tree", { > - "since": $last_update_token, Just change this line to: $last_update_line I.e. you don't need to duplicate the whole query just to omit/include a single line in it, or am I missing something? (This suggestion *would* include a redundant line, but I'm assuming JSON/watchman deals with that just fine...).
Hi Ævar, On Tue, Jun 7, 2022 at 10:42 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Tue, Jun 07 2022, Son Luong Ngoc wrote: > > > In rare circumstances where the current git index does not carry the > > last_update_token, the fsmonitor v2 hook will be invoked with an > > empty string which would caused the final rendered json to be invalid. > > > > ["query", "/path/to/my/git/repository/", { > > "since": , > > "fields": ["name"], > > "expression": ["not", ["dirname", ".git"]] > > }] > > > > Which will left user with the following error message > > > > > git status > > failed to parse command from stdin: line 2, column 13, position 67: unexpected token near ',' > > Watchman: command returned no output. > > Falling back to scanning... > > > > Hide the "since" field in json query when "last_update_token" is empty. > > > > Signed-off-by: Son Luong Ngoc <sluongng@gmail.com> > > --- > > templates/hooks--fsmonitor-watchman.sample | 21 +++++++++++++-------- > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample > > index 14ed0aa42d..b4ee86dfc4 100755 > > --- a/templates/hooks--fsmonitor-watchman.sample > > +++ b/templates/hooks--fsmonitor-watchman.sample > > @@ -79,6 +79,12 @@ sub watchman_query { > > or die "open2() failed: $!\n" . > > "Falling back to scanning...\n"; > > > > + my $query = <<" END"; > > + ["query", "$git_work_tree", { > > + "fields": ["name"], > > + "expression": ["not", ["dirname", ".git"]] > > + }] > > + END > > Wouldn't a more minimal & obvious patch here be.... > > > # In the query expression below we're asking for names of files that > > # changed since $last_update_token but not from the .git folder. > > # > > @@ -87,15 +93,14 @@ sub watchman_query { > > # output to file names only. Then we're using the "expression" term to > > # further constrain the results. > > if (substr($last_update_token, 0, 1) eq "c") { > > - $last_update_token = "\"$last_update_token\""; > > To just change this to be: > > # same as now: > $last_update_token = "\"$last_update_token\""; > $last_update_line = qq["since": $last_update_token,]; > > Of course having declared the new $last_update_line variable earlier, then: > Yup, I think this is a sensible suggestion. I will fixup and send a V2 shortly. > > + $query = <<" END"; > > + ["query", "$git_work_tree", { > > + "since": "$last_update_token", > > + "fields": ["name"], > > + "expression": ["not", ["dirname", ".git"]] > > + }] > > + END > > } > > - my $query = <<" END"; > > - ["query", "$git_work_tree", { > > - "since": $last_update_token, > > Just change this line to: > > $last_update_line > > I.e. you don't need to duplicate the whole query just to omit/include a > single line in it, or am I missing something? > > (This suggestion *would* include a redundant line, but I'm assuming > JSON/watchman deals with that just fine...). I think we can remove that redundant line by adding '\n' before $last_update_line. I will be including this into the next version. Thanks, Son Luong.
diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample index 14ed0aa42d..b4ee86dfc4 100755 --- a/templates/hooks--fsmonitor-watchman.sample +++ b/templates/hooks--fsmonitor-watchman.sample @@ -79,6 +79,12 @@ sub watchman_query { or die "open2() failed: $!\n" . "Falling back to scanning...\n"; + my $query = <<" END"; + ["query", "$git_work_tree", { + "fields": ["name"], + "expression": ["not", ["dirname", ".git"]] + }] + END # In the query expression below we're asking for names of files that # changed since $last_update_token but not from the .git folder. # @@ -87,15 +93,14 @@ sub watchman_query { # output to file names only. Then we're using the "expression" term to # further constrain the results. if (substr($last_update_token, 0, 1) eq "c") { - $last_update_token = "\"$last_update_token\""; + $query = <<" END"; + ["query", "$git_work_tree", { + "since": "$last_update_token", + "fields": ["name"], + "expression": ["not", ["dirname", ".git"]] + }] + END } - my $query = <<" END"; - ["query", "$git_work_tree", { - "since": $last_update_token, - "fields": ["name"], - "expression": ["not", ["dirname", ".git"]] - }] - END # Uncomment for debugging the watchman query # open (my $fh, ">", ".git/watchman-query.json");
In rare circumstances where the current git index does not carry the last_update_token, the fsmonitor v2 hook will be invoked with an empty string which would caused the final rendered json to be invalid. ["query", "/path/to/my/git/repository/", { "since": , "fields": ["name"], "expression": ["not", ["dirname", ".git"]] }] Which will left user with the following error message > git status failed to parse command from stdin: line 2, column 13, position 67: unexpected token near ',' Watchman: command returned no output. Falling back to scanning... Hide the "since" field in json query when "last_update_token" is empty. Signed-off-by: Son Luong Ngoc <sluongng@gmail.com> --- templates/hooks--fsmonitor-watchman.sample | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)