diff

diff

[Tobin C. Harding]

On occasions diff output does not render the same as it does when
editing a file. For example, while removing checkpatch tab warning
the following diff was generated

-#define KS_WLAN_SET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 4
-#define KS_WLAN_GET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 5
+#define KS_WLAN_SET_WPS_ENABLE         SIOCIWFIRSTPRIV + 4
+#define KS_WLAN_GET_WPS_ENABLE         SIOCIWFIRSTPRIV + 5

However when editing the file the alignment was not changed, just tabs
were inserted instead of spaces.

I had the same thing happen a few days ago (reproduced here from
memory)

-static void foo(struct bar *barp, struct baz *bazp)
+static void foo(struct bar *barp,
+               struct baz *bazp)
{
        ...

When editing the file the two s's in struct were aligned but not in
the diff output. I reapplied this patch to a clean tree and it applied
correctly.

How do maintainers visually parse this if the output is not identical in the
diff and the file once applied?

thanks,
Tobin.

diff

[Greg KH]

On Mon, Mar 13, 2017 at 05:57:14PM +1100, Tobin C. Harding wrote:
> On occasions diff output does not render the same as it does when
> editing a file. For example, while removing checkpatch tab warning
> the following diff was generated
> 
> -#define KS_WLAN_SET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 4
> -#define KS_WLAN_GET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 5
> +#define KS_WLAN_SET_WPS_ENABLE         SIOCIWFIRSTPRIV + 4
> +#define KS_WLAN_GET_WPS_ENABLE         SIOCIWFIRSTPRIV + 5
> 
> However when editing the file the alignment was not changed, just tabs
> were inserted instead of spaces.
> 
> I had the same thing happen a few days ago (reproduced here from
> memory)
> 
> -static void foo(struct bar *barp, struct baz *bazp)
> +static void foo(struct bar *barp,
> +               struct baz *bazp)

You missed that there are really tabs in the real diff, you used all
spaces here.

> {
>         ...
> 
> When editing the file the two s's in struct were aligned but not in
> the diff output. I reapplied this patch to a clean tree and it applied
> correctly.
> 
> How do maintainers visually parse this if the output is not identical in the
> diff and the file once applied?

We are used to reading diffs :)

we can edit them by hand as well, but that's a skill best left alone...

greg k-h

diff

[Nicholas Mc Guire]

On Mon, Mar 13, 2017 at 05:57:14PM +1100, Tobin C. Harding wrote:
> On occasions diff output does not render the same as it does when
> editing a file. For example, while removing checkpatch tab warning
> the following diff was generated
> 
> -#define KS_WLAN_SET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 4
> -#define KS_WLAN_GET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 5
> +#define KS_WLAN_SET_WPS_ENABLE         SIOCIWFIRSTPRIV + 4
> +#define KS_WLAN_GET_WPS_ENABLE         SIOCIWFIRSTPRIV + 5
> 
> However when editing the file the alignment was not changed, just tabs
> were inserted instead of spaces.

if the removed lines were using tabs which they should then they are
moved by the inserted "-" while the added lines were using spaces
and thus did not "jump" to the next tabstop.

> 
> I had the same thing happen a few days ago (reproduced here from
> memory)
> 
> -static void foo(struct bar *barp, struct baz *bazp)
> +static void foo(struct bar *barp,
> +               struct baz *bazp)
> {
>         ...
> 
> When editing the file the two s's in struct were aligned but not in
> the diff output. I reapplied this patch to a clean tree and it applied
> correctly.

this is simply because the insertion is by tabs so when diff added the
+/- output it moved the "static void foo..." but not the "struct baz"
so this looks unaligned in the diff output while it actually is aligned
properly.

> 
> How do maintainers visually parse this if the output is not identical in the
> diff and the file once applied?
>
checkpatch will warn if spaces were used and if its based on tabs
I guess most can see if aligment is correct or not.

thx!
hofrat

diff

[Tobin C. Harding]

On Mon, Mar 13, 2017 at 07:43:00AM +0000, Nicholas Mc Guire wrote:
> On Mon, Mar 13, 2017 at 05:57:14PM +1100, Tobin C. Harding wrote:
> > On occasions diff output does not render the same as it does when
> > editing a file. For example, while removing checkpatch tab warning
> > the following diff was generated
> > 
> > -#define KS_WLAN_SET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 4
> > -#define KS_WLAN_GET_WPS_ENABLE                 SIOCIWFIRSTPRIV + 5
> > +#define KS_WLAN_SET_WPS_ENABLE         SIOCIWFIRSTPRIV + 4
> > +#define KS_WLAN_GET_WPS_ENABLE         SIOCIWFIRSTPRIV + 5
> > 
> > However when editing the file the alignment was not changed, just tabs
> > were inserted instead of spaces.
> 
> if the removed lines were using tabs which they should then they are
> moved by the inserted "-" while the added lines were using spaces
> and thus did not "jump" to the next tabstop.
> 
> > 
> > I had the same thing happen a few days ago (reproduced here from
> > memory)
> > 
> > -static void foo(struct bar *barp, struct baz *bazp)
> > +static void foo(struct bar *barp,
> > +               struct baz *bazp)
> > {
> >         ...
> > 
> > When editing the file the two s's in struct were aligned but not in
> > the diff output. I reapplied this patch to a clean tree and it applied
> > correctly.
> 
> this is simply because the insertion is by tabs so when diff added the
> +/- output it moved the "static void foo..." but not the "struct baz"
> so this looks unaligned in the diff output while it actually is aligned
> properly.
> 
> > 
> > How do maintainers visually parse this if the output is not identical in the
> > diff and the file once applied?
> >
> checkpatch will warn if spaces were used and if its based on tabs
> I guess most can see if aligment is correct or not.

Nice explanation, thank you.

Regards,
Tobin.