[RFC] Looking for opinions on accepting refactoring patches

[Joey Pabalinas <joeypabalinas@gmail.com>]

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

Recently I've become interesting in contributing to Zsh, and
have already been submitting a few scattered patches here and
there, but I will admit it's been a pretty trying experience
overall.

The reason is that many parts of the Zsh codebase are literally
_byzantine_labyrinths_ of single-letter identifiers and uncommented
shenanigans that were very confusing to figure out.

It seems I'm not the only one with difficulties understanding many
parts of the code, as a quick `git grep` for things like 'why' '??'
and 'what' reveal gems like:

> Src/Zle/computil.c:2850: * -->PLEASE DON'T ASK<--

> Src/Zle/complist.c:3431: * *** PLEASE DON'T ASK ME WHY THIS IS NECESSARY ***

> Test/E01options.ztst:879: # Goodness only knows why.

> Src/glob.c:2488: /* please do not laugh at this code. */

> Src/params.c:4167: /* ??? error checking */

> Src/hist.c:3196: * However, I'm so confused it could simply be baking Bakewell tarts.

> Src/Zle/computil.c-4240-
>
> * As there are virtually no comments in this file, I don't really
> * know why we're doing this, but it's to do with a matcher which
> * is passed as an argument to the utility compfiles -p/-P.

> Src/Zle/zle_tricky.c-1080-
>
> /* Lasciate ogni speranza.
> This function is a nightmare.  It works, but I'm sure that nobody really *
> understands why.  The problem is: to make it cleaner we would need       *
> changes in the lexer code (and then in the parser, and then...).         */

Most of the offending code is part of the completion subsystem, and
obviously being one of the central parts of Zsh, I can understand the
reluctance on changing anything but what is absolutely needed to
to avoid breaking things.

But in my opinion this is probably why I haven't noticed as many people
contributing to Zsh as I expect to (it really is an absolutely *amazing*
piece of software engineering in my opinion), so I was interested in
some comments about how patches concentrated only on refactoring the
code with semantics changes whatsoever would be received?

Is this something that would be encouraged? Or is the risk of bugs
and regression just too heavy for this to be a realistic goal?

Quite honestly I find either answers to be an understandable position,
thus I figured it would be better to ask about it before attempting any
patches like this.

-- 
Cheers,
Joey Pabalinas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]