* On 5/19/20 11:24 PM, Melroy van den Berg wrote:
> As Mike suggested, I redone all changes (plus even some more bonus items).
> Since I was in a separate git branch I could run the format-patch easily via:git
> format-patch master. Thanks for the suggestion.
> 
> @Uli I'm using the auto-formatter for C++ built-in in VS Code.
> 
> I think the end-result with all the separate patches makes more sense now, sorry
> I didn't do it right away. I think once I have a wiki account, I will add all
> these best practices your x2go wiki. Instead of Attachment: <your-patch.diff>:
> https://wiki.x2go.org/doku.php/wiki:bugs?s[]=git&s[]=patch#bug_submission_with_a_patch.
> 
> See the attachments for all the patches in this mail (11 in total). Thanks in
> advance.

Alex's coding style is... special, I guess, but I usually don't change it unless
I have to change a line anyway and mine can also be considered odd at times.


0001-First-auto-format-the-code-for-readability-and-human.patch: won't apply,
it's just adding unnecessary noise and making blaming more difficult.


0002-endl-for-every-debug-line.patch: won't apply. Using (std::)endl is actually
a bug, because qDebug (which is used by x2goDebug internally) already appends a
newline character by default, so another endl call would just give us double
newlines and make the log output worse. I'm actually tempted to remove the other
endls instead, but there's no real benefit in doing so immediately, so I'll just
keep them until a change to the lines has to be made anyway (or it fits
semantically).


0003-send-eof-before-close.patch: good catch, thanks, applied. Strictly
speaking, though, this should not cause any issues, as ssh_channel_close () will
(according to its documentation) already send an EOF. The worst thing that
SHOULD happen is that the additional ssh_channel_send_eof () call is unnecessary
and will cause an internal error in libssh because it might want to write to an
already EOF'd and closed channel. Then again, this error should be benign, since
we'll kill the channel right away anyway.


0004-return-false-in-all-bad-cases.patch: applied, thanks.


0005-Refactor-and-fix-channel-session-closes.patch: highly amended, but mostly
ultimately applied. Did not apply unnecessary code style or endl changes, but
picked most stuff. Also, I originally made the new function static because I
didn't see any benefit in having it part of the API for other places to call it.
Turned out this was a bad idea, so I reverted to registering it within the class
and make it private, like you did. Thanks.


0006-Remove-deprecated-call-and-use-new-libssh-API-call.-.patch: won't apply.
This code path is for old libssh versions only (pre-0.8) and still works there,
so I don't see any reason to change it. Newer versions should already use the
non-deprecated function, so everything's fine.


0007-Fix-when-not-password-auth.-Otherwise-you-get-double.patch: mostly applied,
slightly amended. Not changing return (...) vs. return ..., the actual change
looks good though.


0008-Correct-retval-check.-Plus-some-minor-formatting.patch: functionally
applied, minus formatting.


0009-Another-missing-endl.patch: not applied, same reasoning as for 0001.


0010-Mark-as-unused-explicitly.patch: can't hurt, I guess, so applied, but
slightly amended.


0011-improve-gitignore-why-not.patch: mostly applied, without the .gitignore
file entry deletion which is intentional.



Mihai