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

expand $left/$right in edit-args, replace all $variable matches #1211

Merged
merged 6 commits into from
Feb 7, 2023
Merged

expand $left/$right in edit-args, replace all $variable matches #1211

merged 6 commits into from
Feb 7, 2023

Conversation

yuja
Copy link
Collaborator

@yuja yuja commented Feb 6, 2023

Checklist

If applicable:

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

Copy link
Collaborator

@ilyagr ilyagr left a comment

Choose a reason for hiding this comment

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

Thanks Yuya, this is nice! I have three kinds of comments:

-- Some changes I'd like to see in the documentation. If you don't feel like doing them here, feel free to leave them for me to do in another PR.

-- I would rather not change all the default configs to explicitly include $left $right, unless you feel strongly about that.

-- Some minor take-it-or-leave-it thoughts about the code.

src/merge_tools.rs Outdated Show resolved Hide resolved
src/merge_tools.rs Outdated Show resolved Hide resolved
src/merge_tools.rs Outdated Show resolved Hide resolved
docs/config.md Show resolved Hide resolved
src/merge_tools.rs Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
docs/config.md Show resolved Hide resolved
docs/config.md Show resolved Hide resolved
The default edit_args will be changed to ["$left", "$right"] to support
variable substitution without breaking the existing configuration too much.

The default merge_args could also be set if we could come up with something
meaningful.
…strings

The assumption here is temp_dir wouldn't contain invalid utf-8 bytes. If it
can contain invalid bytes, maybe we can remove temp_dir from arguments, and
chdir(temp_dir) instead.

This unblocks the use of Regex. We could use regex::bytes, but it's way
more complex as we would have to go back and forth between str/OsStr and
bytes.
I'll add string interpolation support to edit-args.
And set edit_args = ["$left", "$right"] by default.
@yuja yuja merged commit ba1c4f5 into martinvonz:main Feb 7, 2023
@yuja yuja deleted the push-31266528fe3d branch February 7, 2023 09:32
@yuja yuja mentioned this pull request Feb 7, 2023
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