Smalltalk › Pharo › Pharo Smalltalk Developers

Idea and proof of concept: Accept for Test

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

18 messages Options

Marcus Denker-4

Idea and proof of concept: Accept for Test

Hi,

Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.

It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
does not change, yet, when running tests, the version of the code we accepted for testing is executed.

So how do we implement that?

1) menu in the browser. I did a quick hack, a Suggestions AST menu shows for all nodes (to be replaced with the “correct” way later).

SugsSuggestion subclass: #SugsAcceptForTest
instanceVariableNames: ''
classVariableNames: ''
package: 'SmartSuggestions-Suggestion’

label
^'Accept for test’

So, now we can do everything in the #execute method.

What do we need?

1) How we know that we are in a test? —> CurrentExecutionEnvironment value isTest
2) can we compile the buffer even if there are syntax errors (kind of what I expect to happen in non-accepted code)?
—> yes, remember, we use the same Parser for syntax highlighting and we extended code-generation for SyntaxError Nodes
in the past (it raises a syntax error at runtime, isn’t that kind off odd in a fun way?).

newMethod := context selectedClass compiler
source: context code;
options: #(+ optionParseErrors);
compile.

3) Now how to we “hook into” the execution of the original unchanged method? We use a MetaLink. We install it as a before link on
the RBMethodNode, the meta-object is a block. This block gets as arguments the thisContext and the arguments of the original method
and then just does what you want it do do: if we are in test, return the result of the execution of the method we just compiled.
(Closures are fun, we can just reference the outer temp newMethod, no need to store it anywhere else explicitly):

[ :aContext :args |
CurrentExecutionEnvironment value isTest
ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];

Thus, all the code is this:

execute
| newMethod metaLink |
“we compile a new method from the not-accepted text editor. Allow syntax errors"
newMethod := context selectedClass compiler
source: context code;
options: #(+ optionParseErrors);
compile.
"the link executes the method we just created and returns"
metaLink := MetaLink new
metaObject: [ :aContext :args |
CurrentExecutionEnvironment value isTest
ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
selector: #value:value:;
arguments: #(context arguments).
context selectedMethod ast link: metaLink

Cleanup we do not care as everything will be GCed as soon as we accept the method for real.

And it works! I did not test it too much, but it seems to do what it is supposed to do. (but more tests and testing are for sure needed)
https://github.com/pharo-project/pharo/pull/1182

Next step: move it to a better place in the menu.

Marcus

Sven Van Caekenberghe-2

Re: Idea and proof of concept: Accept for Test

Very cool. Impressive combination of unique Pharo features.

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
> So how do we implement that?
>
> 1) menu in the browser. I did a quick hack, a Suggestions AST menu shows for all nodes (to be replaced with the “correct” way later).
>
> SugsSuggestion subclass: #SugsAcceptForTest
> instanceVariableNames: ''
> classVariableNames: ''
> package: 'SmartSuggestions-Suggestion’
>
> label
> ^'Accept for test’
>
> So, now we can do everything in the #execute method.
>
> What do we need?
>
> 1) How we know that we are in a test? —> CurrentExecutionEnvironment value isTest
> 2) can we compile the buffer even if there are syntax errors (kind of what I expect to happen in non-accepted code)?
> —> yes, remember, we use the same Parser for syntax highlighting and we extended code-generation for SyntaxError Nodes
> in the past (it raises a syntax error at runtime, isn’t that kind off odd in a fun way?).
>
> newMethod := context selectedClass compiler
> source: context code;
> options: #(+ optionParseErrors);
> compile.
>
> 3) Now how to we “hook into” the execution of the original unchanged method? We use a MetaLink. We install it as a before link on
> the RBMethodNode, the meta-object is a block. This block gets as arguments the thisContext and the arguments of the original method
> and then just does what you want it do do: if we are in test, return the result of the execution of the method we just compiled.
> (Closures are fun, we can just reference the outer temp newMethod, no need to store it anywhere else explicitly):
>
> [ :aContext :args |
> CurrentExecutionEnvironment value isTest
> ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
>
> Thus, all the code is this:
>
> execute
> | newMethod metaLink |
> “we compile a new method from the not-accepted text editor. Allow syntax errors"
> newMethod := context selectedClass compiler
> source: context code;
> options: #(+ optionParseErrors);
> compile.
> "the link executes the method we just created and returns"
> metaLink := MetaLink new
> metaObject: [ :aContext :args |
> CurrentExecutionEnvironment value isTest
> ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
> selector: #value:value:;
> arguments: #(context arguments).
> context selectedMethod ast link: metaLink
>
> Cleanup we do not care as everything will be GCed as soon as we accept the method for real.
>
> And it works! I did not test it too much, but it seems to do what it is supposed to do. (but more tests and testing are for sure needed)
> https://github.com/pharo-project/pharo/pull/1182
>
> Next step: move it to a better place in the menu.
>
> Marcus
>
>

Nicolas Cellier

Re: Idea and proof of concept: Accept for Test

Yes this is cool.

I was wondering what if you try and modify the Meta-Link architecture itself, but my brain did not reach this second meta level...

2018-04-04 21:50 GMT+02:00 Sven Van Caekenberghe <[hidden email]>:

Very cool. Impressive combination of unique Pharo features.

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
> So how do we implement that?
>
> 1) menu in the browser. I did a quick hack, a Suggestions AST menu shows for all nodes (to be replaced with the “correct” way later).
>
> SugsSuggestion subclass: #SugsAcceptForTest
> instanceVariableNames: ''
> classVariableNames: ''
> package: 'SmartSuggestions-Suggestion’
>
> label
> ^'Accept for test’
>
> So, now we can do everything in the #execute method.
>
> What do we need?
>
> 1) How we know that we are in a test? —> CurrentExecutionEnvironment value isTest
> 2) can we compile the buffer even if there are syntax errors (kind of what I expect to happen in non-accepted code)?
> —> yes, remember, we use the same Parser for syntax highlighting and we extended code-generation for SyntaxError Nodes
> in the past (it raises a syntax error at runtime, isn’t that kind off odd in a fun way?).
>
> newMethod := context selectedClass compiler
> source: context code;
> options: #(+ optionParseErrors);
> compile.
>
> 3) Now how to we “hook into” the execution of the original unchanged method? We use a MetaLink. We install it as a before link on
> the RBMethodNode, the meta-object is a block. This block gets as arguments the thisContext and the arguments of the original method
> and then just does what you want it do do: if we are in test, return the result of the execution of the method we just compiled.
> (Closures are fun, we can just reference the outer temp newMethod, no need to store it anywhere else explicitly):
>
> [ :aContext :args |
> CurrentExecutionEnvironment value isTest
> ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
>
> Thus, all the code is this:
>
> execute
> | newMethod metaLink |
> “we compile a new method from the not-accepted text editor. Allow syntax errors"
> newMethod := context selectedClass compiler
> source: context code;
> options: #(+ optionParseErrors);
> compile.
> "the link executes the method we just created and returns"
> metaLink := MetaLink new
> metaObject: [ :aContext :args |
> CurrentExecutionEnvironment value isTest
> ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
> selector: #value:value:;
> arguments: #(context arguments).
> context selectedMethod ast link: metaLink
>
> Cleanup we do not care as everything will be GCed as soon as we accept the method for real.
>
> And it works! I did not test it too much, but it seems to do what it is supposed to do. (but more tests and testing are for sure needed)
> https://github.com/pharo-project/pharo/pull/1182
>
> Next step: move it to a better place in the menu.
>
> Marcus
>
>

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

> On 4 Apr 2018, at 22:41, Nicolas Cellier <[hidden email]> wrote:
>
> Yes this is cool.
> I was wondering what if you try and modify the Meta-Link architecture itself, but my brain did not reach this second meta level...
>

for now all Links are ignored that are put in the MetaLink itself… but that was more a bootstrap thing. Now that we added more fine-grained
control and better recession stopping, this can for sure be simplified.

But for “meta use” to put a knot in the brain, the next level is not that, but instead “just” using links to install links at runtime or Links that enable/disable
other links.

Marcus

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

In reply to this post by Sven Van Caekenberghe-2

> On 4 Apr 2018, at 21:50, Sven Van Caekenberghe <[hidden email]> wrote:
>
> Very cool. Impressive combination of unique Pharo features.
>

It shows, too, that “doing an experiment on top” and then writing a paper is one thing, but that true progress happens if you can actually integrate the results back into the system.

Marcus

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

> On 5 Apr 2018, at 09:09, Marcus Denker <[hidden email]> wrote:
>
>
>
>> On 4 Apr 2018, at 21:50, Sven Van Caekenberghe <[hidden email]> wrote:
>>
>> Very cool. Impressive combination of unique Pharo features.
>>
>
> It shows, too, that “doing an experiment on top” and then writing a paper is one thing, but that true progress happens if you can actually integrate the results back into the system.
>

And it leads immediately (at least for me) that I start to get lots of ideas…

so now we accept a single method for test.

Why not… (random ideas)

-> For un-accepted code: Why not do something like this automatically at every keystroke? That is, after syntax highlight, we can just
compile a method and store it as a method property on the original method (cost almost nothing).

Then all those for an “implicit layer" that e.g we can very easily activate for the dynamic extended of the exection of an expression
in the playground. You could even open a playground that default to that. There can be a button in the browser “run not accepted”.

-> extend it so that you actually “accept for layer X”, that is, you give a name to groups of these.
-> and of course implement it that the whole group an be accepted as an atomic operation

Marcus

Holger Freyther

Re: Idea and proof of concept: Accept for Test

> On 5. Apr 2018, at 08:31, Marcus Denker <[hidden email]> wrote:
>
>
> And it leads immediately (at least for me) that I start to get lots of ideas…
>
> so now we accept a single method for test.
>
> Why not… (random ideas)

nice! Do we have this for collecting testcases and expected output as well? The tricky part might be to put the Class Under Test into the right state (and condense all duplicate test cases).

holger

Stephane Ducasse-3

Re: Idea and proof of concept: Accept for Test

In reply to this post by Marcus Denker-4

you should use that as an example in the reflectivity paper.

On Wed, Apr 4, 2018 at 6:02 PM, Marcus Denker <[hidden email]> wrote:

> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
> So how do we implement that?
>
> 1) menu in the browser. I did a quick hack, a Suggestions AST menu shows for all nodes (to be replaced with the “correct” way later).
>
> SugsSuggestion subclass: #SugsAcceptForTest
> instanceVariableNames: ''
> classVariableNames: ''
> package: 'SmartSuggestions-Suggestion’
>
> label
> ^'Accept for test’
>
> So, now we can do everything in the #execute method.
>
> What do we need?
>
> 1) How we know that we are in a test? —> CurrentExecutionEnvironment value isTest
> 2) can we compile the buffer even if there are syntax errors (kind of what I expect to happen in non-accepted code)?
> —> yes, remember, we use the same Parser for syntax highlighting and we extended code-generation for SyntaxError Nodes
> in the past (it raises a syntax error at runtime, isn’t that kind off odd in a fun way?).
>
> newMethod := context selectedClass compiler
> source: context code;
> options: #(+ optionParseErrors);
> compile.
>
> 3) Now how to we “hook into” the execution of the original unchanged method? We use a MetaLink. We install it as a before link on
> the RBMethodNode, the meta-object is a block. This block gets as arguments the thisContext and the arguments of the original method
> and then just does what you want it do do: if we are in test, return the result of the execution of the method we just compiled.
> (Closures are fun, we can just reference the outer temp newMethod, no need to store it anywhere else explicitly):
>
> [ :aContext :args |
> CurrentExecutionEnvironment value isTest
> ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
>
> Thus, all the code is this:
>
> execute
> | newMethod metaLink |
> “we compile a new method from the not-accepted text editor. Allow syntax errors"
> newMethod := context selectedClass compiler
> source: context code;
> options: #(+ optionParseErrors);
> compile.
> "the link executes the method we just created and returns"
> metaLink := MetaLink new
> metaObject: [ :aContext :args |
> CurrentExecutionEnvironment value isTest
> ifTrue: [ aContext return: (newMethod valueWithReceiver: aContext receiver arguments: args) ] ];
> selector: #value:value:;
> arguments: #(context arguments).
> context selectedMethod ast link: metaLink
>
> Cleanup we do not care as everything will be GCed as soon as we accept the method for real.
>
> And it works! I did not test it too much, but it seems to do what it is supposed to do. (but more tests and testing are for sure needed)
> https://github.com/pharo-project/pharo/pull/1182
>
> Next step: move it to a better place in the menu.
>
> Marcus
>
>

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

In reply to this post by Marcus Denker-4

Denis Kudriashov

Re: Idea and proof of concept: Accept for Test

Hi Marcus.

I commented on github.

2018-05-16 18:34 GMT+03:00 Marcus Denker <[hidden email]>:

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
here is a version done using a Calypso command:

https://github.com/pharo-project/pharo/pull/1372

missing:
-we need to have some visual clue in the footer
- a tool that lists all “accepted for test” method with the possibility to “commit” all at once.

We will have it almost for free with Calypso as soon as this new metalink will be managed similar way as breakpoints.

"Commit" is of course needs to be implemented.

Marcus

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

On 16 May 2018, at 18:21, Denis Kudriashov <[hidden email]> wrote:

Hi Marcus.
I commented on github.

2018-05-16 18:34 GMT+03:00 Marcus Denker <[hidden email]>:

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
here is a version done using a Calypso command:

https://github.com/pharo-project/pharo/pull/1372

missing:
-we need to have some visual clue in the footer
- a tool that lists all “accepted for test” method with the possibility to “commit” all at once.

We will have it almost for free with Calypso as soon as this new metalink will be managed similar way as breakpoints.
"Commit" is of course needs to be implemented.

I really wonder if this is the same as breakpoint. E.g. it only applies to the whole method, it is not an “action”, so a guter icon

seems to be not the right visualization.

To me the fact that it uses a meta-link is purely an implementation artefact, other than for breakpoints, watchpoints, where we annotate

the AST with actions…

Marcus

Denis Kudriashov

Re: Idea and proof of concept: Accept for Test

2018-05-17 10:53 GMT+03:00 Marcus Denker <[hidden email]>:

On 16 May 2018, at 18:21, Denis Kudriashov <[hidden email]> wrote:

Hi Marcus.
I commented on github.

2018-05-16 18:34 GMT+03:00 Marcus Denker <[hidden email]>:

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
here is a version done using a Calypso command:

https://github.com/pharo-project/pharo/pull/1372

missing:
-we need to have some visual clue in the footer
- a tool that lists all “accepted for test” method with the possibility to “commit” all at once.

We will have it almost for free with Calypso as soon as this new metalink will be managed similar way as breakpoints.
"Commit" is of course needs to be implemented.

I really wonder if this is the same as breakpoint. E.g. it only applies to the whole method, it is not an “action”, so a guter icon
seems to be not the right visualization.

To me the fact that it uses a meta-link is purely an implementation artefact, other than for breakpoints, watchpoints, where we annotate
the AST with actions…

I only speak about management implementation. The way how registries of breakpoints and watchpoints are maintained and used by tools.

They all needs some kind of reification of meta information to allow users distinguish between them.

Even if we will avoid all these caches/registries, we will still need mechanizm to detect that this given metalink is breakpoint and not watchpoint.

So I just want to have reusable mechanizm to manage all these kinds of meta information which is driven by metalinks.

To me "MethodCodeForTest" is just another kind of them. Tools will need to detect them in methods and to distinguish them from breakpoints and others.

Marcus

Denis Kudriashov

Re: Idea and proof of concept: Accept for Test

In reply to this post by Marcus Denker-4

2018-05-17 10:53 GMT+03:00 Marcus Denker <[hidden email]>:

On 16 May 2018, at 18:21, Denis Kudriashov <[hidden email]> wrote:

Hi Marcus.
I commented on github.

2018-05-16 18:34 GMT+03:00 Marcus Denker <[hidden email]>:

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
here is a version done using a Calypso command:

https://github.com/pharo-project/pharo/pull/1372

missing:
-we need to have some visual clue in the footer
- a tool that lists all “accepted for test” method with the possibility to “commit” all at once.

We will have it almost for free with Calypso as soon as this new metalink will be managed similar way as breakpoints.
"Commit" is of course needs to be implemented.

I really wonder if this is the same as breakpoint. E.g. it only applies to the whole method, it is not an “action”, so a guter icon
seems to be not the right visualization.

Hm, I would like to see indication in method table that some methods have "uncommited" changes. But it is not necessary should be icon. It can be star prefix in name like in dirty tabs.

To me the fact that it uses a meta-link is purely an implementation artefact, other than for breakpoints, watchpoints, where we annotate
the AST with actions…

Marcus

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

On 17 May 2018, at 11:46, Denis Kudriashov <[hidden email]> wrote:

2018-05-17 10:53 GMT+03:00 Marcus Denker <[hidden email]>:
On 16 May 2018, at 18:21, Denis Kudriashov <[hidden email]> wrote:

Hi Marcus.
I commented on github.

2018-05-16 18:34 GMT+03:00 Marcus Denker <[hidden email]>:

> On 4 Apr 2018, at 18:02, Marcus Denker <[hidden email]> wrote:
>
> Hi,
>
> Some code is very “active”, executed all the time, e.g. collections, graphics… or imagine if you work on the compiler or debugger.
>
> It would really be nice if we could test a change before accepting it. Something like “Accept for Test” where magically the original method
> does not change, yet, when running tests, the version of the code we accepted for testing is executed.
>
here is a version done using a Calypso command:

https://github.com/pharo-project/pharo/pull/1372

missing:
-we need to have some visual clue in the footer
- a tool that lists all “accepted for test” method with the possibility to “commit” all at once.

We will have it almost for free with Calypso as soon as this new metalink will be managed similar way as breakpoints.
"Commit" is of course needs to be implemented.

I really wonder if this is the same as breakpoint. E.g. it only applies to the whole method, it is not an “action”, so a guter icon
seems to be not the right visualization.

Hm, I would like to see indication in method table that some methods have "uncommited" changes. But it is not necessary should be icon. It can be star prefix in name like in dirty tabs.

Yes, an indication is important (with some UI to just forget the non-accepted code).

Could be a prefix in the tab… or/and something in the footer? In the footer I guess it would be easy to add a menu with some options (browse all un-accepted, accept, undo)

Marcus

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

In reply to this post by Denis Kudriashov

To me the fact that it uses a meta-link is purely an implementation artefact, other than for breakpoints, watchpoints, where we annotate
the AST with actions…

I only speak about management implementation. The way how registries of breakpoints and watchpoints are maintained and used by tools.
They all needs some kind of reification of meta information to allow users distinguish between them.
Even if we will avoid all these caches/registries, we will still need mechanizm to detect that this given metalink is breakpoint and not watchpoint.

So I just want to have reusable mechanizm to manage all these kinds of meta information which is driven by metalinks.
To me "MethodCodeForTest" is just another kind of them. Tools will need to detect them in methods and to distinguish them from breakpoints and others.

I will first do the infrastructure for the breakpoints/watchpoints, then we see if it can be generalised.

Marcus

Tim Mackinnon

Re: Idea and proof of concept: Accept for Test

You guys are awesome!

Sent from my iPhone

On 17 May 2018, at 11:01, Marcus Denker <[hidden email]> wrote:

To me the fact that it uses a meta-link is purely an implementation artefact, other than for breakpoints, watchpoints, where we annotate
the AST with actions…

I only speak about management implementation. The way how registries of breakpoints and watchpoints are maintained and used by tools.
They all needs some kind of reification of meta information to allow users distinguish between them.
Even if we will avoid all these caches/registries, we will still need mechanizm to detect that this given metalink is breakpoint and not watchpoint.

So I just want to have reusable mechanizm to manage all these kinds of meta information which is driven by metalinks.
To me "MethodCodeForTest" is just another kind of them. Tools will need to detect them in methods and to distinguish them from breakpoints and others.

I will first do the infrastructure for the breakpoints/watchpoints, then we see if it can be generalised.

Marcus

Denis Kudriashov

Re: Idea and proof of concept: Accept for Test

In reply to this post by Marcus Denker-4

2018-05-17 13:00 GMT+03:00 Marcus Denker <[hidden email]>:

I really wonder if this is the same as breakpoint. E.g. it only applies to the whole method, it is not an “action”, so a guter icon
seems to be not the right visualization.

Hm, I would like to see indication in method table that some methods have "uncommited" changes. But it is not necessary should be icon. It can be star prefix in name like in dirty tabs.

Yes, an indication is important (with some UI to just forget the non-accepted code).

Could be a prefix in the tab… or/and something in the footer? In the footer I guess it would be easy to add a menu with some options (browse all un-accepted, accept, undo)

Yes. And such method can open editor always in dirty state. Many options.

Another issue is how to work with breakpoints and others.

What the target method for them? original or attached one?

Marcus

Marcus Denker-4

Re: Idea and proof of concept: Accept for Test

On 17 May 2018, at 12:59, Denis Kudriashov <[hidden email]> wrote:

2018-05-17 13:00 GMT+03:00 Marcus Denker <[hidden email]>:

I really wonder if this is the same as breakpoint. E.g. it only applies to the whole method, it is not an “action”, so a guter icon
seems to be not the right visualization.

Hm, I would like to see indication in method table that some methods have "uncommited" changes. But it is not necessary should be icon. It can be star prefix in name like in dirty tabs.

Yes, an indication is important (with some UI to just forget the non-accepted code).

Could be a prefix in the tab… or/and something in the footer? In the footer I guess it would be easy to add a menu with some options (browse all un-accepted, accept, undo)

Yes. And such method can open editor always in dirty state. Many options.

Another issue is how to work with breakpoints and others.
What the target method for them? original or attached one?

oh… good question. As we recompile, I guess that old breakpoints should not be active, but one should be able to define new ones… (but how to visualise that?).

If one thinks this through, what we really want is maybe a full-blown “Layer” system…

But: this is a lot of work. With even more open Questions :-)

Marcus