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

feat: use watchman's watch-project if available #390

Closed
wants to merge 1 commit into from

Conversation

mr-salty
Copy link

If watchman is bersion 3.3 or later, use watch-project rather than
the deprecated watch. The main advantage is eliminating redundant
nested watches (i.e. where one is a subtree of another), which reduces
RAM and CPU usage in the watchman daemon and eliminates the indexing
delay when invoking Command-T after moving down the directory hierarchy.

Although watch-project was introduced in 3.1 it is awkward/inefficient
to use without the relative_root query which was introduced in 3.3.
3.3 was released in June 2015, so most users should have a newer version
but backward compatibility with older versions is maintained.

Fixes #389

Tested:

  • Tested the new code under various scenarios by invoking Command-T in my homedir
    and various subdirectories, as well as other paths (/tmp).
  • Manually tested the fallback path to watch, which is unchanged from existing code.
  • Tested various version numbers by manually simulating the output of --version
    • If the version number is missing or malformed, it will end up as version 0.0
    • test cases: nil, "", "foo.bar", "-5.2", "1", "3.2", 3.3.449", "4", "2022.06.13.00"

If watchman is bersion 3.3 or later, use `watch-project` rather than
the deprecated `watch`. The main advantage is eliminating redundant
nested watches (i.e.  where one is a subtree of another), which reduces
RAM and CPU usage in the `watchman` daemon and eliminates the indexing
delay when invoking Command-T after moving down the directory hierarchy.

Although `watch-project` was introduced in 3.1 it is awkward/inefficient
to use without the `relative_root` query which was introduced in 3.3.
3.3 was released in June 2015, so most users should have a newer version
but backward compatibility with older versions is maintained.

Fixes wincent#389

Tested:
* Tested the new code under various scenarios by invoking Command-T in my homedir
  and various subdirectories, as well as other paths (/tmp).
* Manually tested the fallback path to `watch`, which is unchanged from existing code.
* Tested various version numbers by manually simulating the output of `--version`
    * If the version number is missing or malformed, it will end up as version 0.0
    * test cases: nil, "", "foo.bar", "-5.2", "1", "3.2", 3.3.449", "4", "2022.06.13.00"
@wincent
Copy link
Owner

wincent commented Jun 18, 2022

Thanks @mr-salty — this looks pretty good to me. I'm going to pull it down and run with it locally for a while to validate it before merging.

@wincent
Copy link
Owner

wincent commented Jun 18, 2022

Just documenting this for posterity — it seems that the project switched to date-based version numbers in the move from v4.9.0 to v2020.05.11.0, so the major.to_i > 3 check is going to produce the desired result for both date-based and typical major.minor.patch formats.

@wincent
Copy link
Owner

wincent commented Jun 18, 2022

Fun fact: there is a list-capabilities command that would be great for detecting relative_root support, but it was only added in 3.8 🤣.

@wincent wincent closed this in 81dba1e Jun 18, 2022
@wincent
Copy link
Owner

wincent commented Jun 18, 2022

Thanks for this @mr-salty. Seems to work well enough, and I think the risk is pretty low because most people aren't actually using the Watchman scanner, so I merged this.

Padmamanickam added a commit to Padmamanickam/command-t that referenced this pull request Feb 6, 2023
Closes: wincent/command-t#390

* pull/390:
  docs: update AUTHORS and HISTORY section
  refactor: make .watchmanconfig a valid JSON file
  style: apply comment edits and remove an unnecessary semi-colon
  perf: use watchman's `watch-project` if available
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.

use watch-project rather than deprecated watch
2 participants