Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

Hi,

I see undo was added ~6 years ago, however, it is still disabled by default
:(
The lack of undo can easily be the top priority issue for the users.

From what I understand, the current implementation is to build a global
history across all test plan elements.

It does not feel right, because the test plan is often a mix of loosely
coupled items (e.g. different thread groups represent completely different
test items),
so it looks like undo/redo should be per-element rather than global.

Just in case: IntelliJ IDEA seems to implement per-file undo/redo, so
ctrl+z (cmd+z) reverts actions in the current file.

What do you think if we make undo per-element?

Vladimir

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

>Nice work for a first shot !

Thanks. I think I'll merge it shortly.

So far it looks good to me, and it invalidates the history as the tree
selection is changed.

Frankly speaking, I have no idea on the generic way to associate the fields
to components yet.
In other words, if we want to associate edit history with components (e.g.
allow the user to switch between components and undo individual actions),
then we need to associate undo/redo history with the component rather with
the edit field.
However, that looks like an invasive change since it would require
JMeterGUIComponent#modifyTestElement (or something like that) to push/pop
the history.

Vladimir

Re: Undo/redo: does anybody use it?

[Philippe Mouawad <ph...@gmail.com>]

Nice work for a first shot !

On Fri, Mar 27, 2020 at 11:49 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:

> I've created a draft undo/redo implementation for text fields:
> https://github.com/apache/jmeter/pull/576
>
> Of course, it needs special treatment when tree selection changes, however,
> even just the very basic ctrl+z
> would be way better than nothing.
>
> Any thoughts?
>
> Vladimir
>


-- 
Cordialement.
Philippe Mouawad.

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

I've created a draft undo/redo implementation for text fields:
https://github.com/apache/jmeter/pull/576

Of course, it needs special treatment when tree selection changes, however,
even just the very basic ctrl+z
would be way better than nothing.

Any thoughts?

Vladimir

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

>a) when component UI is created, we can scan the component hierarchy, and
add the listeners to all JTextField-like objects in the tree
>I'm inclined to #a because changing all new JTextField with new
JMeterTextField would look like changing 100500 files out of thin air.

It looks like there's yet another option: override TextFieldUI property in
Swing's UIDefaults.
Then JMeter could provide its own UI class. Then we could transparently
support undo/redo listeners for all the fields, including plugin-provided
GUI classes.

Vladimir

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

>Indeed, undo is only within current active selection.

Ok. I just thought we could start with `undo for edit fields within the
current tree selection`.

In other words, reset all undo history when the test plan element is
re-selected.
I guess we could be able to release that activated by default, so
people could start using undo/redo.

A bit of a problem with that is it is required to attach undo listeners to
all the fields, and the fields can even be created dynamically :-/

I see two approaches:
a) when component UI is created, we can scan the component hierarchy, and
add the listeners to all JTextField-like objects in the tree
b) require that all JTextField must pass through a special API to add undo
listeners

Any thoughts on that?
I'm inclined to #a because changing all new JTextField with new
JMeterTextField would look like changing 100500 files out of thin air.

Vladimir

Re: Undo/redo: does anybody use it?

[Philippe Mouawad <ph...@gmail.com>]

On Tue, Mar 24, 2020 at 1:53 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:

> >Scripting elements like JSR223 and all elements using RSyntaxTextArea
> >already support this.
>
> Not quite.
>
> Suppose the following:
> 1) Create JSR223 Sampler
> 2) Type "hello" into the script field
> 3) Switch to Test Plan
> 4) Switch back to JSR223 Sampler
>
> ctrl+z does nothing. I would expect that history should persist across
> switching between elements.
>
Indeed, undo is only within current active selection.

>
> Vladimir
>


-- 
Cordialement.
Philippe Mouawad.

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

>Scripting elements like JSR223 and all elements using RSyntaxTextArea
>already support this.

Not quite.

Suppose the following:
1) Create JSR223 Sampler
2) Type "hello" into the script field
3) Switch to Test Plan
4) Switch back to JSR223 Sampler

ctrl+z does nothing. I would expect that history should persist across
switching between elements.

Vladimir

Re: Undo/redo: does anybody use it?

[Philippe Mouawad <ph...@gmail.com>]

On Tue, Mar 24, 2020 at 1:21 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:

> sebb>For example, even detecting whether the test plan has been changed
>
> I don't see why change detection is needed for undo.
>
> Philippe>Do you think it will be feasible to implement per-element
> undo/redo
> ?
> Philippe>ok by me anyway to have something in this field
>
> Yet another approach could be `per-field` undo. In other words, `ctrl+z`
> could undo the edits in the currently focused field.
>

Scripting elements like JSR223 and all elements using RSyntaxTextArea
already support this.

I guess user would expect both to be available, but it seems to me more
critical to make elements undo possible first, the amount of work lost is
bigger

>
> Vladimir
>


-- 
Cordialement.
Philippe Mouawad.

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

sebb>My point was that the working copy of the test plan is complicated and
fragile

Well, the solution is to add synchronization (e.g. read-write locks) and
forbid non-synchronized access to the model.
That, however, is an orthogonal story.

Vladimir

Re: Undo/redo: does anybody use it?

[sebb <se...@gmail.com>]

On Tue, 24 Mar 2020 at 12:21, Vladimir Sitnikov
<si...@gmail.com> wrote:
>
> sebb>For example, even detecting whether the test plan has been changed
>
> I don't see why change detection is needed for undo.

My point was that the working copy of the test plan is complicated and fragile.
Undo and redo will need to change the same copy.

> Philippe>Do you think it will be feasible to implement per-element undo/redo
> ?
> Philippe>ok by me anyway to have something in this field
>
> Yet another approach could be `per-field` undo. In other words, `ctrl+z`
> could undo the edits in the currently focused field.
>
> Vladimir

Re: Undo/redo: does anybody use it?

[Vladimir Sitnikov <si...@gmail.com>]

sebb>For example, even detecting whether the test plan has been changed

I don't see why change detection is needed for undo.

Philippe>Do you think it will be feasible to implement per-element undo/redo
?
Philippe>ok by me anyway to have something in this field

Yet another approach could be `per-field` undo. In other words, `ctrl+z`
could undo the edits in the currently focused field.

Vladimir

Re: Undo/redo: does anybody use it?

[sebb <se...@gmail.com>]

I'm sure people would use it if it worked properly.

However, it is far from trivial to implement, as Bugzilla will indicate.

The working copy of the test plan is really complicated, and quite fragile.

For example, even detecting whether the test plan has been changed
since it was loaded does not work 100% of the time -- some test plans
show up as 'dirty' even immediately after loading.

Getting undo working without breaking existing test plans is likely to
be a huge challenge.

On Tue, 17 Mar 2020 at 13:31, Philippe Mouawad
<ph...@gmail.com> wrote:
>
> Hi Vladimir,
> This feature is surely requested by a lot of users, it was not enabled as
> it is a bit weird, you can search for related bugzilla (3 ones if I
> remember).
>
> Do you think it will be feasible to implement per-element undo/redo ?
> ok by me anyway to have something in this field
>
> Regards
>
>
> On Tue, Mar 17, 2020 at 2:24 PM Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> wrote:
>
> > Hi,
> >
> > I see undo was added ~6 years ago, however, it is still disabled by default
> > :(
> > The lack of undo can easily be the top priority issue for the users.
> >
> > From what I understand, the current implementation is to build a global
> > history across all test plan elements.
> >
> > It does not feel right, because the test plan is often a mix of loosely
> > coupled items (e.g. different thread groups represent completely different
> > test items),
> > so it looks like undo/redo should be per-element rather than global.
> >
> > Just in case: IntelliJ IDEA seems to implement per-file undo/redo, so
> > ctrl+z (cmd+z) reverts actions in the current file.
> >
> > What do you think if we make undo per-element?
> >
> > Vladimir
> >
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: Undo/redo: does anybody use it?

[Philippe Mouawad <ph...@gmail.com>]

Hi Vladimir,
This feature is surely requested by a lot of users, it was not enabled as
it is a bit weird, you can search for related bugzilla (3 ones if I
remember).

Do you think it will be feasible to implement per-element undo/redo ?
ok by me anyway to have something in this field

Regards


On Tue, Mar 17, 2020 at 2:24 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:

> Hi,
>
> I see undo was added ~6 years ago, however, it is still disabled by default
> :(
> The lack of undo can easily be the top priority issue for the users.
>
> From what I understand, the current implementation is to build a global
> history across all test plan elements.
>
> It does not feel right, because the test plan is often a mix of loosely
> coupled items (e.g. different thread groups represent completely different
> test items),
> so it looks like undo/redo should be per-element rather than global.
>
> Just in case: IntelliJ IDEA seems to implement per-file undo/redo, so
> ctrl+z (cmd+z) reverts actions in the current file.
>
> What do you think if we make undo per-element?
>
> Vladimir
>


-- 
Cordialement.
Philippe Mouawad.