On 02/23, Stefan Beller wrote: > On Fri, Feb 23, 2018 at 1:30 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > On 02/22, Stefan Beller wrote: > >> > +static enum protocol_version discover_version(struct packet_reader *reader) > >> > +{ > >> ... > >> > + > >> > + /* Maybe process capabilities here, at least for v2 */ > >> > + switch (version) { > >> > + case protocol_v1: > >> > + /* Read the peeked version line */ > >> > + packet_reader_read(reader); > >> > + break; > >> > + case protocol_v0: > >> > + break; > >> > + case protocol_unknown_version: > >> > + die("unknown protocol version: '%s'\n", reader->line); > >> > >> The following patches introduce more of the switch(version) cases. > >> And there it actually is a > >> BUG("protocol version unknown? should have been set in discover_version") > >> but here it is a mere > >> die (_("The server uses a different protocol version than we can > >> speak: %s\n"), > >> reader->line); > >> so I would think here it is reasonable to add _(translation). > > > > This should be a BUG as it shouldn't ever be unknown at this point. And > > I'll also drop that comment. > > Huh? > Then I miss-understood the flow of code. When the server announces its > answer is version 42, but the client cannot handle it, which die call is > responsible for reporting it to the user? > (That is technically a BUG on the server side, as we probably never > asked for v42, so I would not want to print BUG locally at the client?) This is handled in `determine_protocol_version_client(const char *server_response)`, which is just a few lines out of context here. -- Brandon Williams