doc: Naming convention for new QSettings setting names #295

1. 
   hebasto commented at 12:22 pm on April 25, 2021: member

   On master (8f80092d78f758fdb2e79e2a832a0c7a26fa2da1):

    0$ cat ~/.config/Bitcoin/Bitcoin-Qt.conf
    1[General]
    2MainWindowGeometry=@ByteArray(\x1\xd9\xd0\xcb\0\x3\0\0\0\0\ta\0\0\0h\0\0\r\x18\0\0\x2\x86\0\0\ta\0\0\0\x80\0\0\r\x18\0\0\x2\x86\0\0\0\0\0\0\0\0\n\0\0\0\ta\0\0\0\x80\0\0\r\x18\0\0\x2\x86)
    3PeersTabSplitterSizes=@ByteArray(\0\0\0\xff\0\0\0\x1\0\0\0\x2\0\0\x1\x12\xff\xff\xff\xff\0\xff\xff\xff\xff\x1\0\0\0\x1\0)
    4RPCConsoleWindowGeometry=@ByteArray(\x1\xd9\xd0\xcb\0\x3\0\0\0\0\r[\0\0\x1\v\0\0\x10p\0\0\x2\xe7\0\0\r[\0\0\x1#\0\0\x10p\0\0\x2\xe7\0\0\0\0\0\0\0\0\n\0\0\0\r[\0\0\x1#\0\0\x10p\0\0\x2\xe7)
    5RecentRequestsViewHeaderState=@ByteArray(\0\0\0\xff\0\0\0\0\0\0\0\x1\0\0\0\x1\0\0\0\0\x1\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x3\x90\0\0\0\x4\0\x1\x1\x1\0\0\0\0\0\0\0\0\0\0\0\0\x64\0\0\0\x82\0\0\0\x84\0\0\0\0\0\0\0\x4\0\0\0\x9e\0\0\0\x1\0\0\0\0\0\0\0\xb3\0\0\0\x1\0\0\0\0\0\0\x1\xa2\0\0\0\x1\0\0\0\0\0\0\0\x9d\0\0\0\x1\0\0\0\0\0\0\x3\xe8\0\0\0\0\x64)
    6TransactionViewHeaderState=@ByteArray(\0\0\0\xff\0\0\0\0\0\0\0\x1\0\0\0\x1\0\0\0\x2\x1\0\0\0\0\0\0\0\0\0\0\0\x6\x2\0\0\0\x1\0\0\0\x1\0\0\0\x64\0\0\x3\x96\0\0\0\x6\0\x1\x1\x1\0\0\0\0\0\0\0\0\0\0\0\0\x64\0\0\0\x17\0\0\0\x84\0\0\0\0\0\0\0\x6\0\0\0\x64\0\0\0\x1\0\0\0\0\0\0\0\0\0\0\0\x1\0\0\0\0\0\0\0\x64\0\0\0\x1\0\0\0\0\0\0\0\x64\0\0\0\x1\0\0\0\0\0\0\0\xec\0\0\0\x1\0\0\0\0\0\0\x1~\0\0\0\x1\0\0\0\0\0\0\x3\xe8\0\0\0\0\x64)
    7UseEmbeddedMonospacedFont=true
    8addrProxy=127.0.0.1:9050
    9addrSeparateProxyTor=127.0.0.1:9050
   10bPrune=false
   11bSpendZeroConfChange=true
   12fCoinControlFeatures=false
   13fFeeSectionMinimized=true
   14fHideTrayIcon=false
   15fListen=true
   16fMinimizeOnClose=false
   17fMinimizeToTray=false
   18fReset=true
   19fRestartRequired=false
   20fUseNatpmp=false
   21fUseProxy=false
   22fUseSeparateProxyTor=false
   23fUseUPnP=false
   24language=
   25nConfTarget=6
   26nDatabaseCache=450
   27nDisplayUnit=1
   28nFeeRadio=0
   29nPruneSize=2
   30nSettingsVersion=219900
   31nSmartFeeSliderPosition=0
   32nThreadsScriptVerif=0
   33nTransactionFee=1000
   34strDataDir=/home/hebasto/.bitcoin
   35strThirdPartyTxUrls=
   

   The ancient setting names follow Hungarian notation, except for addrProxy, addrSeparateProxyTor, and language.

   Here are the changes since then:

   • MainWindowGeometry and RPCConsoleWindowGeometry were added in https://github.com/bitcoin/bitcoin/pull/11335 (v0.16.0) in replacement of nWindowPos, nWindowSize, nRPCConsoleWindowPos, nRPCConsoleWindowSize

   • PeersTabSplitterSizes was added in #165 (2021-01-07)

   • RecentRequestsViewHeaderState and TransactionViewHeaderState were added in #205 (2021-02-22)

   • UseEmbeddedMonospacedFont was added in #79 (2021-02-22)

   Current suggestions in the open PRs:

   • display_unit in #60 in replace of nDisplayUnit
   • RPCConsoleWindowPeersTabSplitterSizes in #194
   • PeersTabPeerHeaderState and PeersTabBanlistHeaderState in #256

   Some observations:

   1. Widget geometry setting names follow the PascalCase (including open PRs), and the first parts of them point to the appropriate widgets
   2. Other setting names, e.g., display_unit in #60, follow the usual snake_case

   It seems good for future code changes and maintenance to have explicit and documented naming conventions for new QSettings setting names before v22.0 branch off.

   What do you think?

2. hebasto added this to the milestone 22.0 on Apr 25, 2021
3. hebasto added the label Brainstorming on Apr 25, 2021
4. hebasto added the label Doc on Apr 25, 2021
5. hebasto renamed this:
   Naming convention for new QSettings setting names
   doc: Naming convention for new QSettings setting names
   on Apr 25, 2021
6. 
   ryanofsky commented at 1:03 pm on April 26, 2021: contributor

   Saw a ping on IRC about this, but I don’t have strong opinions. Few thoughts:

   • I agree it would be good to have a documented convention for new names, but I don’t think the benefits of renaming existing settings would outweigh the costs. Having seamless forward and backward compatibility is user friendly and developer friendly, and whenever there are multiple names for the same thing it can be more confusing than having a single name, even if the name isn’t perfect.

   • After https://github.com/bitcoin/bitcoin/pull/15936 there is no longer any overlap between node and GUI settings. Node settings are stored in bitcoin.conf/settings.json, and GUI settings like window positions are stored in QSettings. Specifically, the following QSettings are all migrated and removed: nDatabaseCache, nThreadsScriptVerif, bSpendZeroConfChange, fUseUPnP, fUseNatpmp, fListen, nPruneSize, bPrune, addrProxy, fUseProxy, addrSeparateProxyTor, fUseSeparateProxyTor

   • Snake convention, camel case, even hungarian notation all seem reasonable here. (In general I don’t like hungarian notatation, but a vague ini/conf format that doesn’t have a direct way of expressing types is a place where hungarian notation can be useful for self-documentation of settings. I’m thinking of user who might want to tweak the conf file without looking up source or documentation.)

7. 
   hebasto commented at 1:07 pm on April 26, 2021: member

   @ryanofsky Thanks!

   … I don’t think the benefits of renaming existing settings would outweigh the costs.

   True.

8. 
   ryanofsky commented at 1:17 pm on April 26, 2021: contributor

   I think you could open a PR that adds a suggested naming convention for new settings to developer notes and maybe optionsmodel comments. Whatever convention you like should probably be fine, and if others prefer something different they can comment in the PR.
9. hebasto removed this from the milestone 22.0 on Jun 14, 2021

Contributors
hebasto ryanofsky

Labels
Doc Brainstorming