#3142 Issue `closed`: Enable automated source code formatting¶

Labels: waiting for info, discuss / RFC, ReaR Project, no-issue-activity

schlomo opened issue at 2024-01-29 17:42:¶

I'd like to be able to use automated source code formatting with ReaR, and the go-to-tool for that seems to be shfmt which is also supported in our .editorconfig file and in various IDEs (I'm currently using Visual Studio Code with https://marketplace.visualstudio.com/items?itemName=foxundermoon.shell-format).

Our https://github.com/rear/rear/wiki/Coding-Style has some rules that conflict with the abilities of this formatter, specifically adding extra white space or padding around code fragments or before ;.

I wasn't able to find a shell script formatting solution that works in the IDEs that allows such a coding style, and I believe that automated consistent formatting is more important. Therefore I'd like to suggest to adjust our coding style to also allow (or demand?) a more compact way of writing Bash and to recommend using a shell script formatter to format the code.

I'm happy to submit the PR with the change that reformats all lines, although that would kill git blame and therefore I'd rather prefer that we reformat files as we work on them.

Personally I'd prefer to not require a "clean" git blame as a requirement over consistent and automated code formatting.

What do you think @rear/contributors?

jsmeix commented at 2024-02-09 12:00:¶

Personally I don't worry about coding syntax style differences.

For example personally I don't care if

if CONDITION ; then
    ...
    ...
fi

or

if CONDITION
then ...
     ...
fi

is used because both are reasonably easy
to read and - most important - to understand.

Therefore
https://github.com/rear/rear/wiki/Coding-Style
talks initially about "hints" - not "rules":

Here is a collection of coding hints
that should help to get a more consistent code base.

Don't be afraid to contribute to Relax-and-Recover
even if your contribution does not fully match all this coding hints. 
...
Nevertheless try to understand the idea behind this coding hints
so that you know how to break them properly
(i.e. "learn the rules so you know how to break them properly").

The overall idea behind this coding hints is:

Make yourself understood

So as long as it is reasonably easy for me
to understand some code I do not care about
coding syntax style.

For example I prefer spaces as separators
basically everywhere in my code for example as in

if for x in this that ; do echo $x ; done ; then
    echo OK
fi

because personally I find that easier to read
than

if for x in this that; do echo $x; done; then
    echo OK
fi

But this is really not something which makes
the code needlessly hard to read
so I don't worry about such coding syntax style details.

In contrast for example I think that backticks
make code needlessly hard to read - in particular
when also single quotes are used things look confusing.

jsmeix commented at 2024-02-09 12:03:¶

@schlomo
do you perhaps propose
to run an automated source code formatter
over all existing ReaR code?

github-actions commented at 2024-04-10 02:03:¶

Stale issue message

jsmeix commented at 2024-04-10 07:21:¶

See
https://github.com/rear/rear/commit/03ee835853ca800d52249510d23d2c5ed8f8c620#commitcomment-140806662

schlomo commented at 2024-04-10 10:39:¶

@jsmeix the problem we saw just now is actually only a problem of the transition: If all the source code would have been formatted by a tool then whenever somebody works on code the formatting tool ensures that there are no needles formatting changes.

As we have not yet reformatted the ReaR code via tool we keep seeing this problem.

Therefore indeed I propose to decide upon a tool and "convert" the source code with a single PR to that automated formatting.

The main downside I can see to this is that we loose the easy git blame history because then the author of that PR will be visible in all changed lines.

The alternative is IMHO to give up on very consistent ReaR source code formatting and accept PRs that mostly look good without too much attention to the details.

jsmeix commented at 2024-04-10 11:19:¶

My personal preference is to give up on
consistent ReaR source code syntax style and
accept PRs that properly implement actual functionality
without much attention to coding style details
provided the coding style looks acceptable ("cum grano salis")
which means as long as the code is easy to understand
i.e. as long as the code is maintainable which means
that at any time later others still understand the code
so they can properly fix and enhance the code as needed.

Cf. my above
https://github.com/rear/rear/issues/3142#issuecomment-1935804512

jsmeix commented at 2024-04-10 11:21:¶

@gdha @pcahyna
please comment here what your personal opinion is
regarding automated source code formatting and
regarding a consistent ReaR source code syntax style.

schlomo commented at 2024-04-10 11:25:¶

@jsmeix I can also live with accepting different coding styles and formattings, but then I'd expect it to be OK to change the formatting as part of working on code and not be enslaved to the "original" formatting

jsmeix commented at 2024-04-10 11:31:¶

@rear/contributors
when a majority of us active ReaR upstream maintainers
prefer a consistent ReaR source code syntax style
it would be perfectly OK for me to enforce that by
"converting" our whole source code with a single PR
to an automated formatting
because
I do not care much what source code syntax style is used,
cf. my above
https://github.com/rear/rear/issues/3142#issuecomment-1935804512

But if we "convert" our whole source code with a single PR
to an automated formatting then those who want that must
carefully review this PR to avoid that the conversion may
mess up things where "exceptional" syntax stlye is needed.
For example I am thinking about things
like strings over several lines

    my_text="This text
is several
lines long"

where automated indentation would change the string value.

jsmeix commented at 2024-04-10 11:35:¶

@schlomo
yes of course, when you work on some code
(in particular when you overhaul some code),
you become owner of the the new code version
(or at least the one who is to "blame" in git)
and as owner (blamable/culprit) it is your code
where only you decide what syntay style you like
(provided others can understand your code).

I did this very often when I completely overhauled a script
that I used my personally preferred coding style syntax
everywhere in my (then it became 'my') overhauled script.

jsmeix commented at 2024-04-10 11:38:¶

@rear/contributors
when we agreed here how we like to deal
with source code syntax style, I will overhaul
https://github.com/rear/rear/wiki/Coding-Style
appropriately.

gdha commented at 2024-04-10 11:46:¶

@jsmeix @schlomo I'm okay with the Coding-Style of ReaR.

jsmeix commented at 2024-04-10 12:00:¶

Perhaps a good argument against a
consistent ReaR source code syntax style
is that this would cause needless problems
with new contributors who use their own coding style.

I would like to avoid unhelpful discussions about
their own coding style (provided we understand their code)
and focus on whether or not they did properly implement
their intended added/enhanced/changed functionality.

I think that coding style discussions could become
rather quickly very unhelpful because coding style
is a very personal thing so trying to teach others about
their coding style could be soon perceived as intrusive.

I even think that coexisting different coding styles in ReaR
are a good sign of a healty/open/free culture in our project.
In particular when on the other hand we are strict that
we only accept contributions where we clearly understand
what a contribution actually contributes, cf.
https://github.com/rear/rear/issues/3202

Simply put:
When we tell our contributors that their own coding style
does not matter for us but it does matter for us that
we clearly understand what they actually contribute,
such a condition should be willingly accepted
and even appreciated by our contributors.

github-actions commented at 2024-06-10 02:21:¶