[Edbrowse-dev] [patch] Parenthesize assignments used as a truth...

[Edbrowse-dev] [patch] Parenthesize assignments used as a truth...

[Karl Dahlke]

if(t = strchr(line, '@')) {

I have to admit I am a bit hesitant about adding in the extra parentheses

if((t = strchr(line, '@'))) {

and not because of any screen reader.
Rather the parentheses add almost no new information.
They don't make clear the order of operations.
There is no parsing ambiguity.
The line can only run as it runs.
Unlike some of the lines with many operators,
especially the shift and booleans, where the C precedence is really
quite counterintuitive.
Or the ?: with other stuff going on.
I can see adding parentheses to those for clarity.
But to me lines like this seem perfectly clear.
And 2 extra chars, or perhaps 4 after you run indent,
well, if the line is inside some blocks,
4 more chars could push you over 80 and then the damn indenter cuts the line in two,
and then you're reading the logical line of code across two physical lines.
Now that is annoying.
Oh I know, at this point you tell me I have too many
nested blocks, and yeah you might be right,
but it is what it is.
There are a couple of places where it's almost criminal what indent does.

if(z ==
23 ||
y ==
37 ||
q == 1 &&

etc all tabbed over to the right.
Makes me wish I'd stayed with my original 4 space indent instead of the linux kernel
standard 8 space indent.
Or maybe it's what I deserve for nesting 5 blocks.
Anyways I'm a little off topic.
I'll probably hang on to this patch and not push it now,
unless everyone else unanimously thinks I should.

Karl Dahlke

Re: [Edbrowse-dev] [patch] Parenthesize assignments used as a truth...

[Adam Thompson]

[-- Attachment #1: Type: text/plain, Size: 2855 bytes --]

On Fri, Aug 08, 2014 at 07:52:52AM -0400, Karl Dahlke wrote:
> if(t = strchr(line, '@')) {
> 
> I have to admit I am a bit hesitant about adding in the extra parentheses
> 
> if((t = strchr(line, '@'))) {
> 
> and not because of any screen reader.
> Rather the parentheses add almost no new information.
> They don't make clear the order of operations.
> There is no parsing ambiguity.
> The line can only run as it runs.
> Unlike some of the lines with many operators,
> especially the shift and booleans, where the C precedence is really
> quite counterintuitive.
> Or the ?: with other stuff going on.
> I can see adding parentheses to those for clarity.
> But to me lines like this seem perfectly clear.

I agree, but if we want to get rid of warnings then we may as well.

> And 2 extra chars, or perhaps 4 after you run indent,
> well, if the line is inside some blocks,
> 4 more chars could push you over 80 and then the damn indenter cuts the line in two,
> and then you're reading the logical line of code across two physical lines.
> Now that is annoying.

Yep.

> Oh I know, at this point you tell me I have too many
> nested blocks, and yeah you might be right,
> but it is what it is.

I'd rather have lots of nesting than lots of functions created for the single
purpose of removing nested blocks.

> There are a couple of places where it's almost criminal what indent does.
> 
> if(z ==
> 23 ||
> y ==
> 37 ||
> q == 1 &&
> 
> etc all tabbed over to the right.
> Makes me wish I'd stayed with my original 4 space indent instead of the linux kernel
> standard 8 space indent.
> Or maybe it's what I deserve for nesting 5 blocks.

I've always prefered a 4 space indent (well actually I've only started
indenting since I had to start doing Python 3 years ago and even now sometimes I
forget), but at least 4 spaces is enough for me to notice the difference in
indentation and not so much that 5 blocks has eaten half my braille display,
not to mention trying to count 40 spaces with a screen reader or what happens
when I use a 40 cell braille display.
At any rate it's better than a 3 (yep that realy is 3 and not a typo)
spaces as used at the last place I worked.
That basically came as the worst of both worlds between those who wanted 2 and
those who wanted 4 space indentation, and was decided way before my time there.
Getting indent to do that was... fun.
I think I (and those who reviewed my code)
ended up accepting some seriously mangled indentation in the end.

> Anyways I'm a little off topic.
> I'll probably hang on to this patch and not push it now,
> unless everyone else unanimously thinks I should.

I'd say push it on the basis that we're looking for a clean compile and
it'll probably save having this discussion in a few months time.

Cheers,
Adam.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]