refactor: convert checkmake and prettierd from fn to standard config #81
Conversation
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
Problem: most formatters and linters in guard-collection had no test coverage. The csharpier binary was renamed in v1.0+ and ruff's -e flag was deprecated in favor of the check subcommand. Solution: add 42 test files covering every remaining tool, fix the csharpier and ruff definitions, extend the install script with gz/jar archive types, add 6 new CI jobs (dotnet, ruby, clojure, elixir, nix, swift), and expand existing jobs with newly tested tools.
Problem: the -X theirs merge strategy replaced the full test/all-tools CI config with the smaller fix/broken-configs version, losing install entries for dart, fish_indent, google-java-format, pg_format, tombi, typos, typstyle, xmllint, and zigfmt. Solution: restore binary.txt, ci.yaml, and install script from the pre-merge test/all-tools state which already had all entries.
Problem: every test job repeated the same 5 steps for neovim, lua, luarocks, busted/nlua, and guard.nvim clone — 175 lines of duplication across 15 jobs. Solution: extract into .github/actions/test-setup/action.yml and replace with a single `uses: ./.github/actions/test-setup` per job.
Problem: rebase picked up old test versions that manually construct commands instead of using the config-driven helpers, defeating the purpose of testing the actual tool definitions. Solution: replace all 14 affected test files with the upstream/main versions that use run_lint/run_fmt. Add buf lint test using run_lint.
Problem: checkmake and prettierd used custom fn handlers to spawn processes manually, duplicating what guard.nvim's standard cmd/args/parse pipeline already does. This prevented their tests from using the shared run_lint/run_fmt helpers. Solution: replace fn with cmd/args/fname fields. Update tests to use run_lint and run_fmt instead of manual vim.system calls.
Problem: cljfmt required a dedicated CI job with Java, Clojure CLI, a hand-rolled wrapper script, and dep pre-warming. The binary job also had an inline apt-get install command inconsistent with other tool lists. Solution: switch cljfmt to the standalone GraalVM binary from GitHub releases, move its test into test/binary/, delete the test-clojure job, and extract the binary job's apt packages into binary-apt.txt.
Problem: cpplint and zsh tests bypassed the fn config entirely by manually constructing vim.system calls, so drift between the test and the actual config could go undetected. Solution: add run_lint_fn helper that wraps the fn call in a coroutine and pumps the event loop via vim.wait, then update cpplint and zsh tests to use it.
Member
|
I will review after the dependent pr is merged, the diff is to big rn |
Contributor
Author
|
R4R @xiaoshihou514 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Depends on #80
Problem
checkmake and prettierd used custom
fnhandlers to spawn processesmanually, duplicating what guard.nvim's standard
cmd/args/parsepipeline already does. This prevented their tests from using the shared
run_lint/run_fmthelpers.Solution
Replace
fnwith standard config fields (cmd,args,fname,stdin) and update tests to userun_lintandrun_fmt.Create wrapper
run_lint_fnfor the non-standard configs that mocks the coroutines and polls withvim.wait()