Code review comment for lp:~maria-captains/bzr/diffoptions

Revision history for this message
Sergei Golubchik (sergii) wrote :

Hi, Martin!

Thanks for looking into this!

On Mar 21, Martin [gz] wrote:
> + # if the user requested function names in the diff
> + # we use the external diff
>
> My main issue with this kind of change is that actually using an
> external diff rather than the bzr internal one *can* be a big
> behaviour change. Some people don't have an external diff installed at
> all, and encoding handling is completely different.

This is, of course, true. But whether to use this feature is a user's
decision. For example, I suspect that Windowes users (that tend to not
have external diff installed) simply won't set the regex in the
rules file. On the other hand, this feature should work quite good for
lots of users and projects.

> If it's happening in non-obvious circumstances, we risk confusing
> users over when diff works the way they expect and when it breaks.
> Implementing '-F' in the bzr internal differ is way more work, but
> would also do what you want here I think.

Yes, I looked into it too. But a proper solution would require
modifying bzr diff algorithms (both C and python versions, for
consistency) which is not only much more work but also much more risky
approach, as it affects the core functionality of bzr, affects what's
stored in the repository, bundles, everything.

My change, on the other hand, is reasonably small, simple, and safe - it
only affects what is shown to user, all the internal algorithms and data
structures are not affected,

Regards,
Sergei

« Back to merge proposal