xpra icon
Bug tracker and wiki

Changes between Initial Version and Version 1 of Ticket #2041, comment 23


Ignore:
Timestamp:
12/05/18 00:54:50 (9 months ago)
Author:
Antoine Martin
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #2041, comment 23

    initial v1  
    1 Replying to [comment:22 Antoine Martin]:
    2 > It was easier and clearer for me to make the changes rather than to go through another round of review, please see the updated patch - untested, does that work for you?
    3 > Changes:
    4 > * minor import style tweak
    5 > * minor indentation issues (missing whitespace, extra space, etc)
    6 > * logging message cut&pasted wrong: "auth_interactive"
    7 > * re-do WSL workaround which was removed by the patch
    8 > * {{{sshpass_command}}} discarded assignment
    9 > * moved proxy attribute parsing to a function: {{{parse_proxy_attributes}}}
    10 > * re-order proxy attributes to make them consistent with non-proxy attributes: host, port, username, password (and found that {{{proxy_ssh_port_entry}}} was assigned twice that way)
    11 > * {{{is_putty}}} and {{{is_paramiko}}} need to be updated every time the ssh command is updated, not just initially - not sure I like having those two extra attributes, but anyway... (a backend enum might be better here..)
    12 > * used {{{get_username}}} from {{{xpra.platform}}} instead of {{{getpass}}} (should end up the same)
    13 > * renamed "proxy_ssh_port" to  "proxy_port" for consistency / clearer and so that "port" can be reused for other types of proxy
    14 > * rename "proxy_pkey_path" to "proxy_key": we could potentially support inlined base64 encoded keys (like we do for ssl certs) and 'p' is not descriptive
    15 > * renamed "_l" to "_label" for consistency
    16 > * used "%s" string formatting rather than string concatenation
    17 > * tidy up {{{add_ssh_proxy_args}}} a bit
    18 > * make error messages more helpful: show what part of the string parsing failed
    19 >
    20 >
    21 > Other changes still needed / useful:
    22 > * {{{proxy_pkey_path_browse_clicked}}} we should support openssh and paramiko proxy ssh keys
    23 > * {{{nostrict_host_check}}} should only be shown for openssh mode
    24 > * constify modes?
    25 > * split {{{do_ssh_paramiko_connect_to}}}
    26 
    271Thanks!  I'll start testing!