duplicate of #324003 ?

It would be if we only had non-arithmetic operator precedence mode, 
but nowadays, this functionality does make sense for arithmetic operator
precedence mode where it takes the whole arithmetic statement before 
calculating it.

So let's leave it open. Let's even leave it a bug. I've got a feeling
it might even be a simple one line change. I'll investigate it next week.

Sami, can you think of any reason why enabling the display cursor in AOP
mode wouldn't work?

his bug is not a good candidate for use of the "Gnome target" field.

This field is not a 'it would be nice' field, it's a 'Gnome releases may need
to be delayed for this issue' field. It's intended for use by senior-ish bug
triagers and the release team.  Since this bug is not critical enough to delay
a release of the entire desktop the designation has been removed.

The 'Target Milestone' field is meant to be used to describe the version of the
product that developers or the maintainers believe they should fix the bug by.

Difficulties:
 - There is only an append function implemented which adds new input to the end of the existing expression.
 - The input system assumes that the cursor (text injection point) is at the end of the expression after the "=" button is pushed. 

However, I don't think that there are problems that couldn't be solved with a small development cycle.

Okay. I'm reassigning it to you Sami. If you can get it done for GNOME
2.13/2.14 that's great, but it's perfectly okay to wait until the next 
release in order to make sure it's fully tested.

*** Bug 327594 has been marked as a duplicate of this bug. ***

Adjusting summary to reflect that this on only something
that we want to do for aritmetic operator precedence mode.

Created attachment 96466 [details] [review]
Patch that starts to implement this feature.

Patch from Sami for this.

There are currently three problems that remain to be solved:

1) Gcalctool buttons (using mouse) do not insert text at cursor.

2) Undo / Redo works only for calculated expressions (in keyboard
   editing).

3) Writing to the end or beginning uses the default font.

Created attachment 96494 [details] [review]
Updated patch that fixes problem 3).

This patch currently uses "Serif 15". This might not be the best choice.
I need to check on that.

Created attachment 96536 [details] [review]
Revised patch based off of svn trunk (latest).

Created attachment 96543 [details] [review]
Further revised patch to use "default" font for fix for 3).

Will need to remove a line of debug from display_focus_out_cb()
before this gets committed.

Sami, when you have a fix for problem 1), I think we can commit
this and open a new bug for problem 2).

Thanks.

Created attachment 96652 [details] [review]
Revised patch to only allow the display area to be editable in arithmetic precedence mode (if you user toggles the initial setting).

Created attachment 96769 [details] [review]
Further revised patch that partially solves sub-problem #1.

Sami sent me an updated patch. It allows you to use a gcalctool
button via the mouse to add that button's value where the text
caret currently is.

As it stands, this works once, then the text caret is moved to
the end of the line, so the next time you try it, the new value
is inserted there.

Created attachment 96822 [details] [review]
Further revised patch.

Sami sent me some more diffs to try. He's working from an older version
of gcalctool. I had trouble applying these changes to the code at SVN trunk
HEAD. I'm attaching the revised combined patch here, but I haven't committed
it yet. It build and runs, but produces the wrong effect when I try to input
a number via the mouse.

Steps to reproduce:

1. Start gcalctool in arithmetic precedence mode.
2. Use the mouse to give focus to the middle of the number being displayed.
3. Click on the gcalctool button "4".
4. Rather than inserting that number where the cursor is, the display is
   cleared and just "4" is displayed.

Created attachment 96904 [details] [review]
Hopefully the final version of the patch.

This patch has been committed to SVN trunk.

Hopefully this bug has now been fixed. In working on this, we
discovered two other things that have now surfaced:

1/ That Undo/Redo don't work in if the user has been "hand-editing" 
   the display.

2/ That the calculation looses accuracy if the user has been 
   "hand-editing" the display.

I've asked Sami to file bugs on those two problems and start looking
at them.

Thanks.

The patch sets the display GtkTextView to be editable. This is no good (try clicking in it and pressing tab/g/etc). We can still insert text at the cursor/navigate the cursor without the entry being editable.

As per the original report I've connected the home and end keys to select the text buffer (see select_display_entry() in gtk.c).

> We can still insert text at the cursor/navigate 
> the cursor without the entry being editable.

What do you mean by this?



The expression on the display should be editable (keyboard only).

I tried to get the SVN trunk version, but it does seem to compile at the moment.

> I tried to get the SVN trunk version, but it does seem to compile at the
> moment.

Me neither. Here's the compile error:

gtk.c: In function ‘cm_response_cb’:
gtk.c:1478: error: ‘mode’ undeclared (first use in this function)
gtk.c:1478: error: (Each undeclared identifier is reported only once
gtk.c:1478: error: for each function it appears in.)
gtk.c: In function ‘mode_radio_cb’:
gtk.c:2499: warning: implicit declaration of function ‘change_mode’
make[2]: *** [gtk.o] Error 1

From what I have gathered the input behaviour for gcalctool goes as follows:
1. All key events are caught in kframe_key_press_cb() in gtk.c
2. This event is converted into a calculator event (e.g. digit 3, divide, square etc)
3. The calculator event causes the display to be modified.

(this is the same behaviour before and after the Glade patches)

If we make the display entry editable there is different behaviour when the display entry has keyboard focus.

e.g.
When the display entry does not have focus typing 'g' has no effect
When the display entry has focus typing 'g' inserts a 'g' character into the display

and also:
To perform a square root with keyboard focus on the display the user types 'S'-'q'-'r'-'t'-'('
To perform a square root without keyboard focus on the display the user types 's'

If the GtkTextView is marked as non-editable then instead of appending the character we can insert it using ui_insert_display() (was previously called insert_to_cursor()). The current behaviour is that the display content is not handled in gtk.c so I would expect we have a ui_get_cursor() function instead.

In summary if we make the GtkTextView editable as in the current patch we lose all control on what the user can enter and make the behaviour inconsistent depending on if the display has keyboard focus. We can insert text in a user selected location in the display using the cursor value and this does not require making the display editable.

Apologies for the compile error, I don't currently have subversion access at work but what is required is reverting cm_response_cb() and change_mode() to the forms in revision 1793:
http://svn.gnome.org/viewvc/gcalctool/trunk/gcalctool/gtk.c?revision=1793&view=markup

(I will fix the compile error tonight if no-one else has done it before then)

Robert, I like your approach in comment #20. I think it will reduce or
nullify some of the concerns that Sami has outlined in comment #5 of 
bug #488796. I certainly don't think we need to disable the gcalctool
buttons if the display is focused. We only need to use the caret as the
current input location. Please go ahead and see if you can get this working.
Adding support for Home and End (as per the original bug report) would
be required too.

Feel free to fixup the compile error when you get home. I tried earlier,
putting change_mode() back, but then it bitched about one of the routines
that change_mode() called (grey_buttons() I think), so I'll let you work
out what the best thing to do here is.

Thanks.

SVN head compiles again (btw grey_buttons() has been renamed to ui_set_mode() - a little less vague :) )

>Gcalctool should display a
>blinking cursor, and that cursor should move around on the entry line based on
>user input. Home, End, LeftArrow, RightArrow and so forth ought to behave as
>expected.

Robert, it seems the expression can be edited from the beginning and from the end but not from the middle in SVN trunk version of gcacltool?

For example if I have "1+2+3+4+0+6+7+8+9" and I would like to change 0 into 5, it can not be done without deleting either "1+2+3+4" or "6+7+8+9"?

My following comment was somehow gone missing from bugzilla (Date: Mon, 5 Nov 2007 11:39:39 +0200).

>In summary if we make the GtkTextView editable as in the current patch we lose
>all control on what the user can enter and make the behaviour inconsistent
>depending on if the display has keyboard focus. We can insert text in a user
>selected location in the display using the cursor value and this does not
>require making the display editable.

Yes, the idea is to make it possible for user to write and edit expressions as user would write text in a text editor (like most advanced calculators work). We have a parser. It takes care of the syntax.

However, the most important is that we provide the most basic capabilities to user for editing the expression on the display. That is, supporting following operations:
- Set cursor position
- Append at cursor position
- Delete from cursor position (both del and backspace behavior).

Apologies for the reversion, I did not notice the bug comments about it until today. I've got cursor control working on the subversion head again.

I've implemented the behaviour that the inserted text is generated by the key pressed, i.e.:
1. The user presses a calculator key (either using the keyboard or mouse).
2. The key produces a calculator event (e.g. insert '1', add, etc).
3. The result of the event will modify the display.
4. If the display is selected inserted text is placed at the cursor location, otherwise the text is appended to the display.

The advantages of this method is:
1. The behaviour when the display is visually selected is exactly the same as when it is not, i.e. pressing 's' inserts 'Sqrt(' in both cases.
2. The user cannot enter junk data, e.g. tab characters, multibyte strings, use the wrong case etc.

It is very important for the UI to be consistent and to reduce the chance of a user making an error. If the user can type anything then users will be confused when they get "Malformed Expression" errors. Having a parser will mean we can detect the error but avoiding an error is better than detecting it...

However in the future there is the option of taking advantage of having a full keyboard (not generally present on calculators) and giving the user full access to the display. Other PC calculator programs seem to be going in this direction (e.g. Speedcrunch http://speedcrunch.berlios.de/). In doing this we can have more functions than visible keys. In an architecture like this the keys are merely macros for the text.

Moving in this direction is a large change to the current design and should not be embarked upon lightly as it would require restructuring and cause a significant change in UI behaviour. We really need to know who our target audience is and what affect this has on them.

Robert, thanks for looking at the "hand-editing" gcalctool problem again.

I just tried it. You certainly can do the "1+2+3+4+0+6+7+8+9"
change 0 into 5, but if I then hit Return (either with the cursor in
the middle of the line where I made the change, or at the end, I get
a "Malformed expression" error.

This might be a different problem (or a side-effect of the changes
you made) as I notice that clicking on the "1+2=" buttons causes
the same error.

Can I ask a favour in the way you do changes please? Two favours
actually. 

1/ When you make changes, could you add a ChangeLog entry as well.
   Even if you are refixing a problem that was fixed.
 
2/ What would be even better is to also attach a patch to the bug.
   This only makes sense if the fix you are doing is just for that problem.
   The patch is really helpful to let others (including myself), easily see
   how you fixed the problem. What we've also being doing in Orca is
   attaching patches and not checking them in until others have tested
   them. This ensures that SVN trunk doesn't get broken. As we approach
   the next even-number release, we need to ensure that the code
   is stable. Currently in the development cycle, we've hopefully still 
   got plenty of time to find and fix this up before the next release 
   on 3rd December.

As for applications like speedcrunch...

This comes up annually on desktop-devel. qalculate is another such
calculator (http://qalculate.sourceforge.net/). I *strongly* suggest 
that we worry about this when that is what users are demanding for the
default calculator for the GNOME desktop. For now, users can easily
use something else. I see speedcrunch is in the Ubuntu repositories.
Trivial to install.

Thanks!

Robert, I think now the user has the ability to edit the expression in a very consistent way. For this development cycle, your approach seems to be a good one.

However, this consistency gives problems as well. It was somewhat confusing to edit the display and find that the del key erases the whole expression instead of one (following) char. 

>It is very important for the UI to be consistent and to reduce the chance of a
>user making an error. If the user can type anything then users will be confused
>when they get "Malformed Expression" errors. Having a parser will mean we can
>detect the error but avoiding an error is better than detecting it...

I think we can not prevent user from making errors. Generally I don't see a reason even to try to do so.

However, it would be nice to have the parser to point out, to user, where the error is (in the expression).

Rich, I think you need to do a make clean and rebuild. There are some general problems with the build system and the parser which I have reported in bug 497237.

The usual excuse for just committing... It's a case of "doing it properly" (and having the time to do so) or "getting it done" (at 12:45am). I made the judgement call that the current svn behaviour was more broken before (due to my earlier changes) than now. :/

I normally use changelogs to announce new features / fixes between releases (i.e. user visible) and use the subversion history for developers to see changes. We really need to make a page on live.gnome.org with the gcalctool coding style, changelog policy, project direction etc. I'm happy to conform with what the project expects.

Sami, I would argue we can prevent errors... The less inputs (in this case keys) a UI has the less things the user do to screw things up!

I also noticed the delete behaviour and agree it is odd. I think we need to consider changing the clear shortcut to not be delete and then we can use delete as "remove the symbol to the right of the cursor".

> Rich, I think you need to do a make clean and rebuild. There are some general
> problems with the build system and the parser which I have reported in bug
> 497237.

Indeed. Okay. Thanks for filing a new bug on this.

> I normally use changelogs to announce new features / fixes between releases
> (i.e. user visible) and use the subversion history for developers to see
> changes. We really need to make a page on live.gnome.org with the gcalctool
> coding style, changelog policy, project direction etc. 

That's a great idea, especially if we start attracting new developers who
want to use gcalctool to start to get up to speed with GNOME/Gtk+.

> I'm happy to conform with what the project expects.

Thanks. 

> Sami, I would argue we can prevent errors... The less inputs (in this case
> keys) a UI has the less things the user do to screw things up!

I agree. This is a much nicer and cleaner fix than the previous one.

> I also noticed the delete behaviour and agree it is odd. I think we need to
> consider changing the clear shortcut to not be delete and then we can use
> delete as "remove the symbol to the right of the cursor".

I think we need HCI input on this one, before we change it. Adding Calum
to the CC. Calum, with the very latest version of gcalctool in SVN trunk,
it's now possible (in arithmetic precedence mode), to adjust where the
cursor is in the display area and insert new characters. Doing a backspace
works as expected and removes the character to the left of the cursor. We
also currently have Delete at the keyboard accelerator for deleting all
of what's being displayed. But does that make sense anymore? Should it
just delete the character to the right of the cursor, and we have a new
keyboard accelerator for clearing the whole display?

I note that neither Windows Xp or Mac OS X (Tiger) have this ability so
I don't have any precedent to offer.

What do you advice?

Thanks.

Robert, I noticed that the cursor is not returned to the previous location if previous expression is returned to be edited by pressing enter after answer.

For example if I write "1+2+3+4+0+6+7+8+9) and then move cursor to 0 and press enter twice, the cursor is not at 0. It would be nice if the cursor position was restored.

Rich, Mac approach to change keys is quite nice. For example fn+left key is home and fn+right key is end. Perhaps we could use shift+backspace or shift+del or something similar to delete the whole expression instead of del.

Created attachment 99204 [details] [review]
Stores the cursor location in the display history

Fixes the cursor problem reported by Sami.

Do a complete clean before building as this has changed the exprm_state structure.

"Should [Delete] just delete the character to the right of the cursor"

Yes, I would think so.

"and we have a new keyboard accelerator for clearing the whole display?"

Probably.  But...

By adding this new editing freedom, you've also potentially created  a new world of pain :)  Because in effect,  what you've really done is just turned the display into what looks and behaves almost exactly like a regular text field.  This potentially raises the user's expectations to a whole new level-- they might now, for example, reasonably expect Ctrl+A followed by Backspace to clear the input as it does by default in any other text field on the desktop.  But gcalctool uses Ctrl+A for something else.

Add to that the concept of gtk's keyboard themes: if the user has switched to the Emacs keyboard theme, then they would expect Ctrl+A to move the cursor to the beginning of the line instead, and Ctrl+U to clear the display.

So yes, the simple short-term answer would be to change the 'clear display' shortcut to something like Shift+Delete for now-- I'd be happy enough with that.  But longer term, if it looks like a duck and quacks like a duck, then it probably *ought* to behave like a duck...

The original hand-edit feature was to address concerns expressed by Calum. The concerns were basically why widget was given full editing control. Thus it did behave like any text buffer (It did quack like a duck. =))

For the hand-edit to work properly there was need to have "what you see is what you get" expressions. For this reason I originally wanted to take things even further and remove hidden behavior. That is, currently if display is showing, for example, "0.3333" the user can not know if it is trunkated or not. (Multiplying it by *3 might result 1 or it might result 0.9999, depending on how the 0.3333 was produced) Modified behavior was to show "0.3333.." if the number is more accurate than is displayed on the screen. The motivation was actually to make it possible for parser to differentiate the cases. Calum, do you think the notation with two (or three) dots would have been too unfamiliar to users?

However, the approach implemented in SVN head for expression editing works quite well. I am personally quite happy with that.

> For the hand-edit to work properly there was need to have "what you see is what
> you get" expressions. For this reason I originally wanted to take things even
> further and remove hidden behavior. That is, currently if display is showing,
> for example, "0.3333" the user can not know if it is truncated or not.

And bug #485142 is open to fix that problem.

> However, the approach implemented in SVN head for expression editing works
> quite well. I am personally quite happy with that.

And so am I.

The only thing remaining to do is:

"... change the 'clear display' shortcut to something like Shift+Delete 
 for now--"

and make sure that Delete, delete's the character to the right of the
cursor. When that's done and checked in, I'll update the gcalctool
online help to reflect all these changes, and make sure that they are
also included in the gcalctool release notes for GNOME 2.22.

Thanks.

Reassigning to Robert to finish of what needs to be done (coding-wise).

Created attachment 100987 [details] [review]
Makes delete key work in display

Moves clear shortcut from delete to shift+delete
Makes delete key delete the character to the right of the cursor

Code changes complete, reassigning for documentation updating.

Created attachment 101009 [details] [review]
Patch to update the online help.

Adjusted online help documentation to add in a section 4.4. "Changing
The Display Area" and to update the new keyboard shortcut for Clear.

I think this one is now complete. Closing as FIXED. 

Thanks all the help.

Hi,

I tested the SVN trunk. There still seems to be a problem with result accuracy.

For example: If I write 1/3 and press enter the display is showing "0.33333"

Now, if I append *3 (the display is showing 0.33333*3) and press enter the result is 1.

However, if I instead prepend 3* (the display is showing 3*0.33333) the result is 0.99999. 

Thus, the calculator gives different answers depending on how you write the expression. The answers should be same as the expressions are mathematically equivalent.

Sami, that is the expected behaviour, see comment 14 in bug 485142 (which is still open as this is not an adequate solution). This has always been the case with editing the display.