[U-Boot-Users] Patch to clean up syntax highlighting

[U-Boot-Users] Patch to clean up syntax highlighting

[Steven A. Falco]

My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c
file, because it apparently does not parse C ifdef/else/endif.  The following
patch does not change the behavior of the code at all, but does allow the
editor to properly syntax-highlight the file.

Comments invited.

Signed-off-by: Steve Falco <sfalco@harris.com>

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9c5d1fc..1ac91ea 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
 		 * Ethernet Address and serial# can be set only once,
 		 * ver is readonly.
 		 */
+		if (
 #ifdef CONFIG_HAS_UID
 		/* Allow serial# forced overwrite with 0xdeaf4add flag */
-		if ( ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
+		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
 #else
-		if ( (strcmp (name, "serial#") == 0) ||
+		    (strcmp (name, "serial#") == 0) ||
 #endif
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)

[U-Boot-Users] Patch to clean up syntax highlighting

[Jerry Van Baren]

Steven A. Falco wrote:
> My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c
> file, because it apparently does not parse C ifdef/else/endif.  The following
> patch does not change the behavior of the code at all, but does allow the
> editor to properly syntax-highlight the file.
> 
> Comments invited.

OK, you invited comments so I'll be the designated curmudgeon tonight... 
I'm not wild about uglifying our code to accommodating vim syntax 
highlighting limitations.  Perhaps you can fix the vim syntax 
highlighting instead?

> Signed-off-by: Steve Falco <sfalco@harris.com>
> 
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index 9c5d1fc..1ac91ea 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
>  		 * Ethernet Address and serial# can be set only once,
>  		 * ver is readonly.
>  		 */
> +		if (
>  #ifdef CONFIG_HAS_UID
>  		/* Allow serial# forced overwrite with 0xdeaf4add flag */
> -		if ( ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
> +		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
>  #else
> -		if ( (strcmp (name, "serial#") == 0) ||
> +		    (strcmp (name, "serial#") == 0) ||
>  #endif
>  		    ((strcmp (name, "ethaddr") == 0)
>  #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)

I probably should confess I don't use syntax highlighting so that 
probably factors in to my lack of sympathy.

Best regards,
gvb

[U-Boot-Users] Patch to clean up syntax highlighting

[Jerry Van Baren]

Jerry Van Baren wrote:
> Steven A. Falco wrote:
>> My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c
>> file, because it apparently does not parse C ifdef/else/endif.  The following
>> patch does not change the behavior of the code at all, but does allow the
>> editor to properly syntax-highlight the file.
>>
>> Comments invited.
> 
> OK, you invited comments so I'll be the designated curmudgeon tonight... 
> I'm not wild about uglifying our code to accommodating vim syntax 
> highlighting limitations.  Perhaps you can fix the vim syntax 
> highlighting instead?
> 
>> Signed-off-by: Steve Falco <sfalco@harris.com>
>>
>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
>> index 9c5d1fc..1ac91ea 100644
>> --- a/common/cmd_nvedit.c
>> +++ b/common/cmd_nvedit.c
>> @@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
>>  		 * Ethernet Address and serial# can be set only once,
>>  		 * ver is readonly.
>>  		 */
>> +		if (
>>  #ifdef CONFIG_HAS_UID
>>  		/* Allow serial# forced overwrite with 0xdeaf4add flag */
>> -		if ( ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
>> +		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
>>  #else
>> -		if ( (strcmp (name, "serial#") == 0) ||
>> +		    (strcmp (name, "serial#") == 0) ||
>>  #endif
>>  		    ((strcmp (name, "ethaddr") == 0)
>>  #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
> 
> I probably should confess I don't use syntax highlighting so that 
> probably factors in to my lack of sympathy.
> 
> Best regards,
> gvb

Hi Steven,

I got a good night's sleep and a cup of coffee in me and so I'm a little 
more mellow this morning.  ;-)

Looking at the source code, its pretty ugly already.  Your change 
doesn't make it any more ugly and it could be argued that it is slightly 
less ugly (your patch removes the duplication of the "if( (...)" 
statement).  I also don't see any alternative that would make the code 
beautiful.  :-(

Anyway, I formally withdraw my objection to this patch.

Best regards,
gvb

[U-Boot-Users] Patch to clean up syntax highlighting

[Steven A. Falco]

Jerry Van Baren wrote:
>
> Jerry Van Baren wrote:
>> Steven A. Falco wrote:
>>> My text-editor (vim) has a bit of trouble syntax-highlighting the
>>> cmd_nvedit.c
>>> file, because it apparently does not parse C ifdef/else/endif.  The
>>> following
>>> patch does not change the behavior of the code at all, but does
>>> allow the
>>> editor to properly syntax-highlight the file.
>>>
>>> Comments invited.
>>
>> OK, you invited comments so I'll be the designated curmudgeon
>> tonight... I'm not wild about uglifying our code to accommodating vim
>> syntax highlighting limitations.  Perhaps you can fix the vim syntax
>> highlighting instead?
>>
>>> Signed-off-by: Steve Falco <sfalco@harris.com>
>>>
>>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
>>> index 9c5d1fc..1ac91ea 100644
>>> --- a/common/cmd_nvedit.c
>>> +++ b/common/cmd_nvedit.c
>>> @@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
>>>           * Ethernet Address and serial# can be set only once,
>>>           * ver is readonly.
>>>           */
>>> +        if (
>>>  #ifdef CONFIG_HAS_UID
>>>          /* Allow serial# forced overwrite with 0xdeaf4add flag */
>>> -        if ( ((strcmp (name, "serial#") == 0) && (flag !=
>>> 0xdeaf4add)) ||
>>> +            ((strcmp (name, "serial#") == 0) && (flag !=
>>> 0xdeaf4add)) ||
>>>  #else
>>> -        if ( (strcmp (name, "serial#") == 0) ||
>>> +            (strcmp (name, "serial#") == 0) ||
>>>  #endif
>>>              ((strcmp (name, "ethaddr") == 0)
>>>  #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
>>
>> I probably should confess I don't use syntax highlighting so that
>> probably factors in to my lack of sympathy.
>>
>> Best regards,
>> gvb
>
> Hi Steven,
>
> I got a good night's sleep and a cup of coffee in me and so I'm a
> little more mellow this morning.  ;-)
>
> Looking at the source code, its pretty ugly already.  Your change
> doesn't make it any more ugly and it could be argued that it is
> slightly less ugly (your patch removes the duplication of the "if(
> (...)" statement).  I also don't see any alternative that would make
> the code beautiful.  :-(
>
> Anyway, I formally withdraw my objection to this patch.
>
> Best regards,
> gvb
>

Glad to hear it. :-)     

So this is probably my first actual contribution to u-boot.  Is there
something more I should do to have this patch merged into an official
tree, or will the appropriate person pick it up when time permits?

    Steve

[U-Boot-Users] Patch to clean up syntax highlighting

[Haavard Skinnemoen]

On Fri, 13 Jun 2008 08:14:17 -0400
Jerry Van Baren <gerald.vanbaren@ge.com> wrote:

> Looking at the source code, its pretty ugly already.  Your change 
> doesn't make it any more ugly and it could be argued that it is slightly 
> less ugly (your patch removes the duplication of the "if( (...)" 
> statement).  I also don't see any alternative that would make the code 
> beautiful.  :-(

Actually, I think the following would be slightly cleaner:

#ifdef CONFIG_HAS_UID
# define HAS_UID	(1)
#else
# define HAS_UID	(0)
#endif

/* ... */

	if (((strcmp (name, "serial#") == 0)
			&& (!HAS_UID || (flag != 0xdeaf4add)))
		|| ((strcmp (name, "ethaddr") == 0)

(I may have miscounted the parentheses though...I certainly see the
need for working syntax highlighting on this code ;-)

Haavard

[U-Boot-Users] Patch to clean up syntax highlighting

[Wolfgang Denk]

In message <48515BDA.8080404@harris.com> you wrote:
> 
> My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c
> file, because it apparently does not parse C ifdef/else/endif.  The following
> patch does not change the behavior of the code at all, but does allow the
> editor to properly syntax-highlight the file.
> 
> Comments invited.
> 
> Signed-off-by: Steve Falco <sfalco@harris.com>

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The so-called "desktop metaphor" of today's workstations  is  instead
an  "airplane-seat"  metaphor.  Anyone who has shuffled a lap full of
papers while seated between two portly passengers will recognize  the
difference -- one can see only a very few things at once.
                                                   - Fred Brooks, Jr.