Re: [PATCH] bracket matching extension

[Pavel Roskin <proski gnu org>]

Hello!

> > Sorry, I don't feel safe about applying your patch.
>
> OK, I understand it. I have no more time to send patches for /dev/null,
> so goodbye, and have fun! If somebody interested in, please look after
> my patches!

The problem is, if you make changes that you cannot explain, then other
people will have problems understanding your code.  If they try to improve
the code, they will have to understand the code that you didn't bother to
comment properly.

If you are changing the code, you are supposed to understand at least the
functions you are changing, so you are in the best position to comment
your changes.

In fact, it has happened to me many times that I made some important fixes
while documenting my changes.  Just yesterday, I decided not to apply some
patch while I was writing ChangeLog.  I found a better, less intrusive
solution.

I'm actually trying to lower the entry barrier for the new developers.  
Have properly documented code is part of this effort.  It will make it
easier to understand how the program works, and thus lessen the time
needed to make high quality patches.

Messy code discourages other developers.  I could have applied your patch,
but somebody would have to clean it up some day.  I cannot pass this
responsibility to the future developers, because then they would say
"good-bye", or would not even try to improve the code.

The question about REDRAW_PAGE was not random.  Just recently, we had some
problems with redrawing the top line after Ctrl-Y, and that problem only
happened when switching to the subshell and back.  In other words, simply
testing Ctrl-Y would not be enough to find the problem.

In the preceding condition, you replaced edit->bracket with
edit->bracket_this.  Note that the old edit->bracket corresponds to
edit->bracket_other after the patch.  I know that it's very hard to test
if the redrawing is correct and optimal.  There are dozens of other
factors that trigger the redraw process.  But in some circumstances, your
change will play some role.

Again, a recent example.  The "Abort" button was missing in the copy
progress dialog when 'Verbose operation' was disabled.  In fact, the
button was not redrawn.  The fix was deeply in the common dialog code.  
However, it only affected that rare situation, because in other cases
there were some other reasons to redraw that button.

The easiest way to avoid such problems in the future is to write correct 
code today.  I hope you understand.

-- 
Regards,
Pavel Roskin