I like making the GUI cleaner but isn't the slash part of the string? https://github.com/bitcoin/bips/blob/master/bip-0014.mediawiki#proposal

It looks like BIP0014 uses the forward slashes as delimiters or separator. One node doesn't have many user agents, so the delimiter is not needed in this case.

They should not be misused beyond what is specified in this section.

• / separates the code-stack
• : specifies the implementation version of the particular stack
• ( and ) delimits a comment which optionally separates data using ;

Displaying a separator or delimiter in the UI is unnecessary and goes against Jakob Nielsen heuristics: https://www.nngroup.com/articles/ten-usability-heuristics/:

• Aesthetic and minimalist design
  Dialogues should not contain information which is irrelevant or rarely needed. Every extra unit of information in a dialogue competes with the relevant units of information and diminishes their relative visibility.

It's a debug window... it's full of information which is irrelevant and rarely needed.

NACK on the concept

@pstratem, I don't think your NACK would be considered sound technical justification. The UI is displaying a delimiter for no reason.

This also affects the RPC getpeerinfo output. So this would be a tiny API change. Not sure if we want this.

I'm neutral on this change itself.
However, if you do this please add the filtering as a postprocessign step to the GUI code only, not to the core P2P code. RPC should stay the same.

On second thought: no, I don't like this. I like the debug window to report exactly the data that goes over the network. This may make the string prettier but makes it less useful too.

@CryptoDJ I think it's a matter of taste which looks better:

• /btcwire:0.4.0/btcd:0.12.0/ or btcwire:0.4.0/btcd:0.12.0
• /Satoshi:0.11.2/ljr:20151118/ or Satoshi:0.11.2/ljr:20151118
• /Satoshi:0.12.0/Knots:20160225/ or Satoshi:0.12.0/Knots:20160225

As the current way is exactly the way how BIP 14 defines it, you can't really say it is "irrelevant or rarely needed". Personallly, I'd suggest we just stick with BIP 14 for simplicity and consistency.

The member variable is called cleanSubVer and it is put through a sanitize function. See the comment in net.h:

// strSubVer is whatever byte array we read from the wire. However, this field is intended
// to be printed out, displayed to humans in various forms and so on. So we sanitize it and
// store the sanitized version in cleanSubVer. The original should be used when dealing with
// the network or wire types and the cleaned string used when displayed or logged.

When consuming the API message, the / in cleanSubVer is kept and displayed to end users. Web sites display it b/c who want to further sanitize the string? Also, why have strSubVer and cleanSubVer?

https://bitnodes.21.co/nodes/
There are hundreds of superfluous forward slashes that got propagated because of a sanitation issue:

I think BIP 14 pertains to the wire format, NOT the UI.

I think BIP 14 pertains to the wire format, NOT the UI.

Correct.

However, the reason for the debug window is troubleshooting. For technical diagnostics, it is important to have accurate information. Granted, removing a slash is not a big deal, but we shouldn't make a habit out of presenting things differently than they are on the wire.

In any case: if the goal of this is to affect the UI only, this pull request should touch only UI code.

This is adding code which is for all intents doing nothing.

NACK on the simple principle that less code is better.

My problem is with BIP 14 and I will pull request to change it. The user-agent string example begins and ends with delimiters (the forward slash or /) that have no use to humans or computers and therefore should be eliminated. Eliminating these superfluous delimiters will slightly decreased the data size transmitted to peers, stored when consuming API messages and make the UI look better.

So far, no one has been able to explain the benefit of the leading and trailing forwards slashes except that it is specified in BIP 14.

Edit: BIP 14 pull request: bitcoin/bips#352

I'm closing this, there doesn't seem to be agreement to do this.