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

Add multi-tool config for jj fix #4079

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Add multi-tool config for jj fix #4079

merged 2 commits into from
Jul 25, 2024

Conversation

hooper
Copy link
Contributor

@hooper hooper commented Jul 12, 2024

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@hooper hooper changed the title Push opqkpoomxpyq Add multi-tool config for jj fix Jul 13, 2024
@hooper hooper force-pushed the push-opqkpoomxpyq branch from bd538e3 to 59b3d99 Compare July 13, 2024 01:05
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
@hooper hooper force-pushed the push-opqkpoomxpyq branch from 59b3d99 to 0eb882e Compare July 16, 2024 15:57
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with jj fix, but the change looks good to me, thanks!

cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/config-schema.json Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/tests/test_fix_command.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

cli/src/commands/fix.rs Outdated Show resolved Hide resolved
cli/src/commands/fix.rs Show resolved Hide resolved
@hooper hooper force-pushed the push-opqkpoomxpyq branch 7 times, most recently from e1e9db2 to 6ad70f9 Compare July 22, 2024 21:23
Copy link
Contributor

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, there's one last Nit though.

cli/src/commands/fix.rs Show resolved Hide resolved
cli/src/config-schema.json Show resolved Hide resolved
@hooper hooper force-pushed the push-opqkpoomxpyq branch 2 times, most recently from 8224c47 to a5d6ea0 Compare July 25, 2024 00:42
hooper and others added 2 commits July 25, 2024 12:32
This allows tests to easily distinguish the effects of multiple formatters that
could be applied during a single `jj fix` command, which is helpful for testing
the feature for configuring multiple formatters on potentially overlapping
filesets.

Uppercase is a counterpart to lowercase. Append exposes the number of changes
and their order of execution in the file content, which provides a terse way of
writing test expectations.
…ilesets

The high level changes include:
 - Reworking `fix_file_ids()` to loop over multiple candidate tools per file,
   piping file content between them. Only the final file content is written to
   the store, and content is no longer read for changed files that don't match
   any of the configured patterns.
 - New struct `ToolsConfig` to represent the parsed/validated configuration.
 - New function `get_tools_config()` to create a `ToolsConfig` from a `Config`.
 - New tests; the only old behavior that has changed is that we don't require
   `fix.tool-command` if `fix.tools` defines one or more tools. The general
   approach to validating the config is to fail early if anything is weird.

Co-Authored-By: Josh Steadmon <[email protected]>
@hooper hooper force-pushed the push-opqkpoomxpyq branch from a5d6ea0 to b403a7e Compare July 25, 2024 17:39
@hooper hooper merged commit 89f5d16 into main Jul 25, 2024
29 checks passed
@hooper hooper deleted the push-opqkpoomxpyq branch July 25, 2024 18:40
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.

3 participants