Add OSX keystroke to clear RPCConsole #10324

spencerlievens commented at 8:04 AM on May 3, 2017: contributor

Currently only Ctrl-L is mentioned in help, but, CMD-L works on OSX and isn't mentioned.

jonasschnelli commented at 8:05 AM on May 3, 2017: contributor

ACK 6315a6fd0689c7469f9c266bf5b4f0e337ef79d4

jonasschnelli added the label GUI on May 3, 2017

in src/qt/rpcconsole.cpp:735 in 6315a6fd06 outdated

 730 | @@ -731,7 +731,8 @@ void RPCConsole::clear(bool clearHistory)
 731 |          );
 732 |  
 733 |      message(CMD_REPLY, (tr("Welcome to the %1 RPC console.").arg(tr(PACKAGE_NAME)) + "<br>" +
 734 | -                        tr("Use up and down arrows to navigate history, and <b>Ctrl-L</b> to clear screen.") + "<br>" +
 735 | +                        tr("Use up and down arrows to navigate history") + "<br>" +
 736 | +                        tr("To clear screen use <b>Ctrl-L</b> on Windows/Linux or <b>⌘CMD-L</b> on OSX") + "<br>" +

laanwj commented at 9:13 AM on May 3, 2017:

I'm not usually a person to say this but: let's make this message platform dependent instead of enumerating the platforms here.

spencerlievens commented at 11:33 AM on May 3, 2017:

@laanwj #ifdef Q_OS_MAC it?

fanquake added the label MacOSX on May 3, 2017

spencerlievens commented at 11:50 AM on May 3, 2017: contributor

Improved the code to use an ifdef for OSX instead.

in src/qt/rpcconsole.cpp:739 in f2bd7d9f6d outdated

 736 | +#ifdef Q_OS_MAC
 737 | +                        tr("Use up and down arrows to navigate history, and <b>⌘cmd-L</b> to clear screen.") + "<br>" +
 738 | +#else
 739 | +                        tr("Use up and down arrows to navigate history, and <b>Ctrl-L</b> to clear screen.") + "<br>" +		
 740 | +#endif
 741 | +			tr("Type <b>help</b> for an overview of available commands.")) +

laanwj commented at 11:51 AM on May 3, 2017:

Possible suggestion:

#ifdef Q_OS_MAC
QString clearKeyStr = "<b>⌘cmd-L</b>";
#else
QString clearKeyStr = "<b>Ctrl-L</b>";
#endif
...
tr("Use up and down arrows to navigate history, and %1 to clear screen.").arg(clearKeyStr) + "<br>" +

After all:

The key is not to be translated
Also gets rid of the HTML formatting in the translation string in one go

jonasschnelli commented at 11:54 AM on May 3, 2017:

If we are already bikeshedding: Alternative suggestion would be adding QString generalFunctionKey() to platformstyles.cpp. Then use something like tr("Use up and down arrows to navigate history, and %1-L to clear screen.").arg(platformStyle->generalFunctionKey())

jonasschnelli commented at 11:55 AM on May 3, 2017:

I guess you can also do arg(""+platformStyle->generalFunctionKey()+"") to avoid html in translatable strings.

spencerlievens commented at 11:58 AM on May 3, 2017:

That is much tidier than my attempt 👍

#ifdef Q_OS_MAC QString clearKeyStr = "⌘cmd-L"; #else QString clearKeyStr = "Ctrl-L"; #endif

laanwj commented at 11:59 AM on May 3, 2017:

Yes, adding it to platformStyle would be another step further, I didn't want to suggest that here to not make life too difficult for @spencerlievens while also not making life harder for translators by adding another message that is almost-the-same.

spencerlievens commented at 12:03 PM on May 3, 2017:

Yeah adding to platform style is certainly the more "correct" path, but induces a lot of work.

spencerlievens commented at 12:41 PM on May 3, 2017: contributor

@laanwj When I find some time I will add to platformstyle.cpp and update the translation files as it isn't difficult, just time consuming.

in src/qt/rpcconsole.cpp:738 in 08caa96a77 outdated

 729 | @@ -730,9 +730,15 @@ void RPCConsole::clear(bool clearHistory)
 730 |              ).arg(fixedFontInfo.family(), QString("%1pt").arg(consoleFontSize))
 731 |          );
 732 |  
 733 | +#ifdef Q_OS_MAC
 734 | +    QString clsKey = "<b>⌘cmd-L</b>";
 735 | +#else
 736 | +    QString clsKey = "<b>Ctrl-L</b>";
 737 | +#endif
 738 | +

paveljanik commented at 8:39 AM on May 5, 2017:

There is a TAB on this otherwise empty line.

spencerlievens commented at 2:13 PM on May 6, 2017:

Done

in src/qt/rpcconsole.cpp:734 in 08caa96a77 outdated

 729 | @@ -730,9 +730,15 @@ void RPCConsole::clear(bool clearHistory)
 730 |              ).arg(fixedFontInfo.family(), QString("%1pt").arg(consoleFontSize))
 731 |          );
 732 |  
 733 | +#ifdef Q_OS_MAC
 734 | +    QString clsKey = "<b>⌘cmd-L</b>";

paveljanik commented at 8:41 AM on May 5, 2017:

Can you please move this HTML down to arg(clsKey)? E.g.

arg("<b>"+clsKey+</b>)

or something and remove it from here?

spencerlievens commented at 2:13 PM on May 6, 2017:

Done

in src/qt/rpcconsole.cpp:741 in 08caa96a77 outdated

 738 | +	
 739 |      message(CMD_REPLY, (tr("Welcome to the %1 RPC console.").arg(tr(PACKAGE_NAME)) + "<br>" +
 740 | -                        tr("Use up and down arrows to navigate history, and <b>Ctrl-L</b> to clear screen.") + "<br>" +
 741 | -                        tr("Type <b>help</b> for an overview of available commands.")) +
 742 | +                        tr("Use up and down arrows to navigate history, and %1 to clear screen.").arg(clsKey) + "<br>" +
 743 | +			tr("Type <b>help</b> for an overview of available commands.")) +

paveljanik commented at 8:42 AM on May 5, 2017:

Spaces replaced by TABs? Why?

spencerlievens commented at 2:08 PM on May 6, 2017:

Urgh that was my editor.

paveljanik changes_requested

paveljanik commented at 8:42 AM on May 5, 2017: contributor

And please squash.

in src/qt/rpcconsole.cpp:740 in 49c1b7b8fd outdated

 736 | +    QString clsKey = "Ctrl-L";
 737 | +#endif
 738 | +
 739 |      message(CMD_REPLY, (tr("Welcome to the %1 RPC console.").arg(tr(PACKAGE_NAME)) + "<br>" +
 740 | -                        tr("Use up and down arrows to navigate history, and <b>Ctrl-L</b> to clear screen.") + "<br>" +
 741 | +                        tr("Use up and down arrows to navigate history, and %1 to clear screen.").arg("<b>"+clsKey+"<b>") + "<br>" +

paveljanik commented at 2:20 PM on May 6, 2017:

spencerlievens commented at 4:45 PM on May 6, 2017: contributor

@paveljanik thats what happens when you edit via mobile and online on GitHub

Add OSX keystroke to clear RPCConsole

Currently only Ctrl-L is mentioned in help, but, CMD-L works on OSX and isn't mentioned.

1f82f3af39

spencerlievens commented at 6:15 PM on May 6, 2017: contributor

@laanwj @jonasschnelli @paveljanik

All squashed and good to go ;)

paveljanik commented at 7:41 PM on May 6, 2017: contributor

In the help of e.g. Terminal and Safari, I see Command (⌘)-L notation.

spencerlievens commented at 9:36 AM on May 7, 2017: contributor

@paveljanik This is true, but I wanted to spell it out for the user and make it clear as day.

Shall we see what the view is from a few heads first before I possibly change and squash?

How about a combination of both? (⌘)cmd-L <---- This looks good in my opinion.

spencerlievens commented at 7:45 PM on May 8, 2017: contributor

I realised that this work was not done on a Bitcoin fork and in fact a fork of Dash, that had been subsequently deleted, the new PR for this is #10362

spencerlievens closed this on May 8, 2017

DrahtBot locked this on Sep 8, 2021