kcalc suggestions

[Raphael Kubo da Costa]

On Thursday 22 April 2010 17:06:01 Eike Krumbacher wrote:
> Hi
> 
> Raphael Kubo da Costa schrieb:
> > Base isn't easy to understand when you first look at it. Isn't it better
> > to use Logic Mode or something like that?
> 
> "Numeral System Mode" is now my favourite. :-)
> 
> > +	const QString calcmode = KCalcSettings::calcMode();
> > I'd rather have an enum here instead of a QString.
> 
> That wasn't easy to find the right documentation about kconfig compiler,
> but now we have enums instead.
> 
> Eike

Hey there,

The patch looks almost OK to me. My remaining concerns are "language"-related. 
Last but not least, do you know if there are bug reports that will be touched 
by this commit (either some report that is going to be fixed, or some report 
that will be reopened)?

Here we go.

Index: kcalc.kcfg
===================================================================
--- kcalc.kcfg  (Revision 1117432)
+++ kcalc.kcfg  (Arbeitskopie)
@@ -101,6 +101,26 @@
       </whatsthis>
       <default>true</default>
     </entry>
+    <entry name="CalcMode" type="Enum">

Were we to follow the API style guidelines, it'd be better to be more explicit 
and avoid abbreviations. In this case, CalcMode would become CalculatorMode.

+      <choices>
+        <choice name="easy">
+          <label>Easy Calculator Mode</label>
+          <whatsthis>A very easy mode where only the basic calculator buttons 
are set</whatsthis>

Is "Easy" a good name? How about "Simple"? As for the whatsthis description, I 
suggest something along the lines "A simple mode in which only the basic 
calculator buttons are shown".

+        </choice>
+        <choice name="science">
+          <label>Science Calculator Mode</label>
+          <whatsthis>Easy mode with science buttons and optional constants 
buttons</whatsthis>

Is it necessary to reference another mode here by mentioning Easy Mode? Isn't 
science buttons too vague?

+        <choice name="numeral">
+          <label>Numeral System Mode</label>
+          <whatsthis>Easy Mode with logic buttons and selectable base. 
Optional bit edit available.</whatsthis>

Heh, why Numeral? :)

Index: kcalc.cpp
===================================================================
--- kcalc.cpp   (Revision 1117432)
+++ kcalc.cpp   (Arbeitskopie)

+       actionModeEasy->setText( i18n("Eas&y Mode") );

I don't remember if you need to set the accelerator character manually. If so, 
don't you also need to set it for the other modes as well?