Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

TaurusValueSpinBox arrows do not work #707

Closed
cmft opened this issue Feb 26, 2018 · 6 comments
Closed

TaurusValueSpinBox arrows do not work #707

cmft opened this issue Feb 26, 2018 · 6 comments
Labels
Milestone

Comments

@cmft
Copy link
Member

cmft commented Feb 26, 2018

There is a regression in TaurusValueSpinBox. Press the arrows does not change the value. It works in 4.1.1

@cmft cmft added the bug label Feb 26, 2018
@cpascual
Copy link
Member

Confirmed.
The following code can be used to test if the code is affected by this bug:

from taurus.qt.qtgui.application import TaurusApplication
from taurus.qt.qtgui.input import TaurusValueSpinBox
import sys
app = TaurusApplication()
w = TaurusValueSpinBox()
model = 'sys/tg_test/1/double_scalar'
w.setModel(model)
a = w.text()
w.stepUp()
b = w.text()
assert (a != b)
print "all ok"

@cpascual
Copy link
Member

cpascual commented Feb 27, 2018

... and using it with git bisect, we find that things got broken in 0aaa3e1 :

taurus(release-Jan18)$ git bisect start release-Jan18 4.1.1          
Bisecting: 176 revisions left to test after this (roughly 8 steps)
[c974aa1106af3aaf18918f134d7c7148bbdfd4ec] Merge remote-tracking branch 'taurus-org/develop' into delayed_subscriber
taurus((c974aa11...)|BISECTING)$ git bisect run python ~/taurusbug707.py
(...)
0aaa3e18104c1e0dadddb54c3579cf1da93d0938 is the first bad commit
(...)
Date:   Thu Jan 18 15:05:20 2018 +0100

    bugfix lineedit with units when model specifies, in fragments, to show only the magnitude

@cpascual cpascual added this to the Jan18 milestone Feb 27, 2018
@cpascual cpascual changed the title TaurusValueSpinBox does not work TaurusValueSpinBox arrows do not work Feb 27, 2018
@cpascual
Copy link
Member

I think I located the problem.

It is in TaurusValueLineEdit.setValue() where a call to displayValue(v) was changed to getDisplayValue(v) in 0aaa3e1. The problem is that while displayValue(v) returns a str representation of v, getDisplayValue() returns a str repr of the current model object value (which in this case it is the old value). In fact the current code is buggy because by passing v to getDisplayValue(), v is interpreted as the cache= arg.... which makes no sense.

I think that the solution should be based on:

  • Fixing setValue for TaurusValueLineEdit
  • and possibly implementing {get,set}Value for TaurusValueSpinBox

@cmft
Copy link
Member Author

cmft commented Feb 28, 2018

Yesterday, I arrived to the same conclusion. But I did not find a good solution:

This can be one, replacing
v_str = str(self.getDisplayValue(v))
by

if self.modelFragmentName and ".magnitude" in self.modelFragmentName:
     v = v.magnitude
 v_str = str(self.displayValue(v))

@cpascual
Copy link
Member

Hi @cmft . The solution that you propose could "work" but as it is it looks a bit fragile to me.
I will propose something similar, but based on displayValue and attending to the following considerations:

  • The fragments support was introduced in Spinbox was showing units even if the model use fragments to display only the magnitude #669 for solving one specific use case: allowing to display the wvalue without units. IMHO supporting fragments generically in TaurusValueLineEdit is problematic (e.g. it makes little sense to use "rvalue" as a fragment since "wvalue" will be modified when eventually applying). Therefore for now we can limit the fragment support in line edits to the "wvalue.magnitude" case.

  • In general, a line edit should leave unaltered any text manually entered by the user. Therefore the fragment (or format!) should only be applied in a line edit in two situations:

    • at initialization
    • when modifying the text via step increments (either by using spinbox arrows or when using the keyboard arrows or mouse wheel on a line edit )

cpascual pushed a commit that referenced this issue Feb 28, 2018
The step up/down feature of TaurusValueLineEdit (e.g. using
the up/down keys or the arrows of a TaurusValueSpinBox) was broken when
merging #669 which implemented fragments support in
TaurusValueLineEdit for allowing the display of the wvalue without units
Now we see that supporting fragments generically in TaurusValueLineEdit
is problematic (e.g. it makes little sense to use "rvalue" as a
fragment since "wvalue" will be modified when eventually applying).
Therefore for now limit the fragment support in line edits
to the "wvalue.magnitude" case.

Also fix a hook error introduced in some previous merge.
@cpascual
Copy link
Member

Closed via 72ecf52

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants