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.
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
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
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.
participants (3)
-
Greg KH -
Nicholas Mc Guire -
Tobin C. Harding