Make some things more solid - #8
Merged
Merged
Conversation
yusufraji
approved these changes
Aug 6, 2026
split-patch.rs will not do anything and not show any output paths for the file, which should be fine.
To avoid that the `re!` macro shares the name of the module, which is fine for the compiler, but rust-analyzer continues to mess up the imports.
…splitting
regex_utils.rs:
- Remove `get_` prefix from method names that panic
- Re-add `get_` methods that now return Option
Add patch/change_line.rs:
- Need fine grained testing, so replace the boolean functions,
separate the parsing from the decision, and make the parsing
complete.
- Move it to another file, with a *change*, not hunk-related name.
patch/hunk.rs:
- Use and parse all captures, keep the original numbers around
(use separate mut start and patched_start variables).
- Report invalid change lines (via `ChangeLineReport::from` ->
`try_take_while` -> `stop_reason.separate_errors()`)
- Verify that the end of the matching is a valid end (`match
end_indicator ...`).
- Update the mut variables *before* exiting the loop, for correct
verification below.
- Verify that the original range lengths are correct (after
ignoring "\..." lines).
Maybe this should all be done when constructing Hunk instead
(leaving it as is to make it lazy, but error reporting is now
kinda inconsistent).
pflanze
force-pushed
the
cj_make_more_solid
branch
from
August 6, 2026 13:08
57ec982 to
f5130c4
Compare
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
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.
I couldn't rebase those changes on top of main, and I'm too lazy to patchtool forth and back and verify that things still work in each situation. Thus I'm pushing this last of my current series of PRs on top of the changes of the other PRs. The following commits are new:
Splitting a patch file that has no diffs should probably be a no-op. (I think the case here excludes patch files with diffs that indicate renames; those would not trigger this code path.) Thus I have changed it to be so.
This could be changed to be dependent on an option, like by default give an error but silence the error; but that should be done universally, thus should think about other cases that might not produce any output files if silenced. (See #13)
I thought I already had that somewhere, seems it got lost. Include it here: make the IDE experience more solid.
head_lineto Hunk to eliminate a potential panic (66d8d9c)Parse, don't validate.
try_take_while(5cadf64)Allow to report errors while splitting or why the split boundary was reached. Necessary for 5.
The Perl script simply ignored the numbers denoting the range sizes in the original changes, as did the Rust version so far. Instead, verify that those are correct. This will be helpful to detect when patches were messed up (while manually editing or similar).
This could warrant an option to ignore such errors (lower to warnings, or disable the check completely). For now I couldn't bother. (See #13)
As mentioned in the commit message, maybe these checks should be done when constructing
Hunkobjects instead, by fully parsing them into changes, but that would mean to change the logic to turn the splitting into parsing and then o reimplement the hunks->changes splitting based on the parsed list of changes. Can't be bothered right now. (And it's cool as is since it's more efficient not to parse when not necessary, that's what I meant with "lazy"; but that shouldn't be the primary concern. And error reporting only happens now when splitting by change boundary, which is weak. Thus, in the future everything should probably be changed to fully parse everything universally. The Perl script was just such an awesome (ehr) hack...)