On 7/27/23 21:22, Eric Blake wrote:
> On Thu, Jul 27, 2023 at 08:50:31PM +0200, Laszlo Ersek wrote:
>>> File "GoLang.ml", line 186, characters 11-71:
>>> 186 | pr ("\t" ^ "/* %s field is ignored unless %sSet == true.
>>> */\n")
>>>
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> Error: This expression has type string but an expression was expected of
>>> type
>>> ('weak2 -> 'weak3 -> 'weak4, unit, string, unit) format4 =
>>> ('weak2 -> 'weak3 -> 'weak4, unit, string, string, string, unit)
>>> CamlinternalFormatBasics.format6
>>
>> Sigh. :/
>>
>> "pr" is declared like this [generator/utils.mli]:
>>
>> val pr : ('a, unit, string, unit) format4 -> 'a
>>
>> The format string would normally be converted to a "format4" type, which
>> is a polymorphic type, taking one type variable ('a).
>
> You dug even deeper than me. Thanks, even if it didn't yield a
> trivial solution.
>
>> Ultimately open-coding the \t escape sequences seems the least intrusive...
>>
>>>
>>> so those are non-starters. We can get more verbose with:
>>>
>>> + pr "\t";
>>> + pr "/* %s field is ignored unless %sSet == true. */\n"
>>> fname fname;
>>>
>>> but that may not scale as nicely. Go's use of TAB does not specify
>>> what tabstop it prefers;
>>
>> well that I find unfathomable; how can any coding style mandate TABs for
>> indentation if it doesn't explain how wide a TAB should be? For example,
>> that makes "maximum line width" mostly impossible to define.
>
> https://go.dev/doc/effective_go#formatting
>
> | Some formatting details remain. Very briefly:
> |
> | Indentation
> | We use tabs for indentation and gofmt emits them by default. Use spaces
> only if you must.
> | Line length
> | Go has no line length limit. Don't worry about overflowing a punched
> card. If a line feels too long, wrap it and indent with an extra tab.
>
> and no mention of tab width. The Go community really expects you to
> use gofmt without questions.
I may be overly sensitive, but I don't appreciate the "punched card"
reference. For work, I use a 22" monitor (landscape orientation), I like
to place two text windows (columns) side by side, and my eyesight isn't
the greatest, so I use relatively large fonts. My preferred source code
width is therefore 80 characters.
Other developers work with 100-120 chars per line; in edk2 we've seen
200+ chars even. When I complain, the answer is "just buy a larger
monitor, or use two monitors". Well, I don't feel *comfortable* using
two monitors (I've tried), *or* with a super wide screen (I've also got
a 24" monitor and that one feels too wide to me already -- but some
people use 49-50" monitors!).
It's strange that the go coding style tries to control the spacing
around operators like "+", but ignores the line length question (and
throws a quasi-insult at people that work with narrow source code). If a
codebase is consistently written with a 128 chars/line limit, it might
very well pass "gofmt", but I'd be mostly incapable of working with it.
>
>>
>>> our .editorconfig suggests using a tabstop of
>>> 4 (instead of the usual 8) when reading a .go file for less visual
>>> distraction, and which matches with GoLang.ml currently using 4-space
>>> indentation.
>>>
>>> Rich, do you have any ideas on any better approach to take here?
>>>
>>>>> - pr " %sSet bool\n" fname;
>>>>> - pr " %s " fname;
>>>>> + pr "\t%sSet bool\n" fname;
>>>>> + pr "\t%s " fname;
>>>
>>> I also debated about splitting this patch into two (yes, more
>>> gruntwork): one to address space before '(', and the other to address
>>> leading indentation. Lines like this would be touched by both
>>> patches if I split.
>>>
>>
>> I wouldn't insist on such a split. :)
>
> The benefit of such a split: changing "int (r)" to "int(r)" is
> non-controversial, while changing " line" to "\tline" could be reverted
> if we find a cleaner way to get pr to do indentation on our behalf.
>
> There's also the idea that if the main thing that gofmt still
> complains about is 4 spaces vs. TAB, we could use coreutils'
> unexpand(1) on platforms where 'gofmt' is not installed (although I'm
> not sure unexpand is portable enough to consider it likely to be
> installed on other systems). The more I think about this, the more
> I'm leaning towards injecting gofmt into the pipeline, especially
> since Tage has already proposed injecting rustfmt into the pipeline.
>
> Still, cleaning up the ' (' to be consistent with language idioms
> seems worthwhile, even if I punt on the TAB issue.
>
OK, sounds like a plan -- "separate out the space removal from before
parens, and integrate gofmt into the build process".
Regarding the new gofmt dependency: can we assume that wherever go
exists, gofmt also exists? (On RHEL9, they are both in golang-bin.) IOW,
whenever we build the go bindings, we can also format them. This seems
to have come up during the discussion, but I don't remember the verdict.
Laszlo
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
