want per-file regexp for enclosing function in diff

Bug #764108 reported by Martin Pool
This bug affects 4 people
Affects Status Importance Assigned to Milestone
In Progress
Martin Pool

Bug Description

Filed retrospectively from <https://code.launchpad.net/~maria-captains/bzr/diffoptions/+merge/54139>:

Let me summarize:
1. It's not always easy to find the "current branch" to be able to get its config.
2. having "-F regex" specified only globally is not flexible enough, as different branches (e.g. one with the C code, another with the Python code) may need different regexps.
2.1 Actually, even one regexp per branch is not enough, as one branch may have both python and C files.
3. Allowing the user to specify arbitrary diff options that affect all generated diffs is too dangerous - "diff -w" will break merge bundles (and "diff -c" breaks bzr shelve and everything that parses diffs).
3.1 But "diff -p" seems to be safe everywhere.

So, I took another approach. It's more strict and more flexible at the same time. More flexible as in the 2.1 above (regexp can be specified per file or a group of files) and more strict as in the 3.1 above (only diff -p - really, diff -F - is allowed).

Related branches

Revision history for this message
Martin Pool (mbp) wrote :

Still to do on this:

 * Ideally, rewrite it so that the function prefixes work with internal diff. Otherwise if this is turned on in a rules file, it's likely to break anyone who checks out that tree on Windows or some other platform without gnu diff, and there might be other side effects.

 * Document the new rule setting in the rules help

 * Also document it in the user guide

 * Spiv mentions the issue of its effect on diffs used in bundle generation, shelves, etc. I think it'll be harmless there, and perhaps even useful to see them inline. It shouldn't break anything, but we should probably at least test it.

Revision history for this message
Martin Pool (mbp) wrote :

Additional points:

* Sergei's branch keys off the rule in the old tree, typically the committed tree. It seems to me it would normally be better to go off the new-side of the diff, which will more often be the working tree and which should make this easier to discover. However, since it doesn't read actually from either tree at the moment, it doesn't matter. So to actually use this you need something in ~/.bazaar/rules

* For this to be most useful, we'd need to also actually read the rules from the tree, which is apparently not done at the moment (though there is some infrastructure). (Which is bug 395731.)

* There's some risk a naive implementation of this will make diff slow even if no rules are configured, by scanning the rules file for every file that differs.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.