bcpierce00 / unison

Unison file synchronizer
GNU General Public License v3.0
4.01k stars 227 forks source link

When synchronizing with SSH, only the first line of a multi-line message from the server is marked as such #797

Open mvglasow opened 1 year ago

mvglasow commented 1 year ago

As mentioned by @tleedjarv in #796, messages from the server should be prefixed with Server: when synchronizing with a remote server over SSH.

I have encountered an instance where this is not the case. To reproduce:

Actual output:

~$ unison someprofile -ui text
Contacting server...
Unison failed: Fatal error: Error in creating unison directory /home/user/.unison:
File exists [mkdir(/home/user/.unison)]
Fatal error: Lost connection with the server

Expected output:

~$ unison someprofile -ui text
Contacting server...
Server: Unison failed: Fatal error: Error in creating unison directory /home/user/.unison:
Server: File exists [mkdir(/home/user/.unison)]
Fatal error: Lost connection with the server

This was on 2.48 on both client and server. (If this has been fixed since, feel free to close.)

tleedjarv commented 1 year ago

Half of the issue is fixed since 2.51.3 where the output reads Unison server failed.

The other half (not fixed) is that File exists [mkdir(/home/user/.unison)] looks like a separate error or a separate output. It is a continuation of the same error. The entire error message is this:

Fatal error: Error in creating unison directory /home/user/.unison:
File exists [mkdir(/home/user/.unison)]

This formatting makes it look like two different errors.

You may want to adjust the issue to be about confusing formatting only.

mvglasow commented 1 year ago

I’ve updated the description.

Suggestion for UX (if not already in place): any message that comes from the server should have a prefix denoting it as such. This prefix should be consistent across all messages (not e.g. Remote host: for one message, Server reports: for another). If a message spans multiple lines, each line should be prefixed.

As an example from the latter, logcat on Android is a good example: anything printed to System.out becomes a logcat entry, and each line is prefixed with a timestamp and severity, even for a message which spans multiple lines.

That means that, rather than just printing Server: followed by the message from the server, Unison would have to:

Output on the client then should be something like:

Contacting server...
Server: Fatal error: Error in creating unison directory /home/user/.unison:
Server: File exists [mkdir(/home/user/.unison)]
Fatal error: Lost connection with the server
tleedjarv commented 1 year ago

Suggestion for UX (if not already in place): any message that comes from the server should have a prefix denoting it as such. This prefix should be consistent across all messages (not e.g. Remote host: for one message, Server reports: for another). If a message spans multiple lines, each line should be prefixed.

I think this is a relevant feature request.

I think it is mainly important with ssh (but not only) because all remote stderr output is printed on local stderr (or in GUI, during startup). Currently with -servercmd nonexistent_executable, for example, you get back an error from your remote shell (something like bash: nonexistent_executable: command not found). This and other similar errors are very confusing for the user.