Download raw body.
got patch: add flag to ignore whitespace?
On 22-07-03 03:28pm, Omar Polo wrote: > Mark Jamsek <mark@jamsek.com> wrote: > > On 22-07-03 12:24am, Christian Weisgerber wrote: > > > Omar Polo: > > > > > Also, regarding "Ignore whitespaces in context lines."; how is "context > > lines" defined? > > patching a file is basically a search and replace operation on lines. > you open the "old file", search around the line address mentioned in the > hunk header a range of lines that matches the hunk, and apply it. > > For a hunk to match it means that all the lines that start with a - or a > space ' ' should match, in order. This is called the context of the > diff. (I'm actually not sure wether "context line" only applies to the > ' ' lines or to both ' ' and '-' lines.) > > > For instance, -w made my diff apply cleanly, but the whitespace was on > > a changed line. > > Your diff failed to apply because the deleted lines were different from > the ones in the file (because of the trailing whitespaces) and so got > patch couldn't find the right offset in the file. After it learned how > to ignore the whitespaces, the context matched and the lines were > changed (althought a few become no-op.) We covered this on irc, but just to recap for posterity: I don't think it's wrong to say "Ignore whitespace in context lines"--I was just looking for how we're defining it in got as some implementations suggest context is just the lines beginning with ' '. > > > If I understand linecmp() correctly, it's like diff -w? > > > > > > Meanwhile patch -l seems to be more like diff -b? > > > > I like Fossil's semantics for this: > > > > -w|--ignore-all-space Ignore white space when comparing lines > > -Z|--ignore-trailing-space Ignore changes to end-of-line whitespace > > > > but I think -w and -W might be better flags than -w and -Z. > > I'm not sure how valuable is to provide both options actually. I think > that the less flag we have, the better. Also, users are really > interested in -w vs -Z? I think they're more interested in got applying > the patch. I agree with the less flags the better. In fact, this is where I love got: it provides all the functionality with a simple interface. That lack of complexity for the user is a stark contrast with some other VCSs. In the case of -w and -Z; I was thinking in terms of how lax we wanted the default to be. But if we're happy with ignoring all ws, then, yes, the one option is fine. > I was going with -w with the same semantics as Fossil here until Stefan > convinced me that there's no point in running the same command twice :) > > > I'm not sure how I feel about this being the default though. I admit, > > it's nice not having failed patches, but as far as I'm aware, no other > > patch implementations do it by default so it might fail the Rule of > > Least Surprise. And--ironically coming from the person who sent out the > > mangled diff that instigated this--it shouldn't happen often. I can't > > even remember the last time I did it. > > I have to agree here, my experience is that there aren't _that_ many > diffs with mangled spaces. There are more diff around with wrapped > lines and perfectly fine diffs that fails to apply due to other changes > in between (fortunately now we have a diff3 merge for patches too.) > > > Plus, with the: > > > > "@@ -295,7 +305,7 @@ hunk contains mangled whitespace" > > > > output, users can then rerun with -w. > > > > That said, I really am ambivalent and can't decide which way I fall on > > this. > > I'm still for having it enabled by default and not giving the option to > turn it off :) The more thought I've given this, the more I'm coming around to stsp's and your position. Invariably, the first thing a user will do after seeing a failed patch with the "hunk contains mangled whitespace" report is run `got patch -w ...` so why not do it the first time? I think I'm sold :) > An argument that can be made thought is that 'got patch' could try > harder before starting to ignore whitespaces. > > At the moment 'got patch' works in a mostly linear fashion: the original > file is traversed from the start and we (almost never) jump back. (in > reality we do some backward jumps, but we don't try as hard as patch(1) > does.) > > I have something in mind for applying diffs with fuzz (i.e. ignoring up > to N lines of context) and I was thinking of doing some changes in the > patch machinery to accomodate multiple passes on the same file. With > that we could move this whitespace-loose matching so it's only done in a > second pass. The idea could be: > > first pass: try to apply the patch, eventually at offset > second pass: try again with a whitespace loose matching > third pass: whitespaces + fuzz > > ideas? (note that it'll take still a while for this changes, can't > provide an eta as i haven't started working on it yet :-) No rush. I haven't had a `got patch` fail yet (despite being the cause of perhaps everyone on the list having had it happen to them :) -- Mark Jamsek <fnc.bsdbox.org> GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
got patch: add flag to ignore whitespace?