enthought / enthought/traitsui

UndoItem merging of sequence types inconsistent

Open
#1,507 0 comments 0 reactions 0 assignees View on GitHub
component: core difficulty: intermediate type: refactor
Dominant language
Python
Stars
306
Forks
99
PR merge metrics
No merged PRs in 30d

Description

The merge method of `UndoItem` for the case of sequence objects returns `True` as long as only one item has changed from the original value:
```
class SimpleExample(HasTraits):
tuple_value = Tuple()

example = SimpleExample()

undo_item = UndoItem(
object=example,
name='tuple_value',
old_value=('foo', 'bar', 'baz'),
new_value=('foo', 'wombat', 'baz'),
)
next_undo_item = UndoItem(
object=example,
name='tuple_value',
old_value=('foo', 'wombat', 'baz'),
new_value=('foo', 'fizz', 'baz'),
)

result = undo_item.merge(next_undo_item)
```
In this case the `result` is `True`.

However, if any of the unchanging items is not a "simple type" (basically strings and numbers) then it always counts as a "changed item", so
```
undo_item = UndoItem(
object=example,
name='tuple_value',
old_value=(['foo'], 'bar', 'baz'),
new_value=(['foo'], 'wombat', 'baz'),
)
next_undo_item = UndoItem(
object=example,
name='tuple_value',
old_value=(['foo'], 'wombat', 'baz'),
new_value=(['foo'], 'fizz', 'baz'),
)

result = undo_item.merge(next_undo_item)
```
returns `False`.

Given that equality tests should work for almost all types, and the block catches all exceptions, it seems safe to simplify the code or otherwise make the behaviour more consistent.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.