See for senders of #errorBlock:, e.g. PushDownInstanceVariableRefactoring, RemoveClassRefactoring or RemoveInstanceRefactoring. On 4 October 2010 15:40, Lukas Renggli <renggli@gmail.com> wrote:

This is used to validate the preconditions of refactorings and inform the user about any mismatches.

It should better not be removed.

Lukas

On 4 October 2010 15:36, Niko Schwarz <niko.schwarz@googlemail.com> wrote:

...

Hi,

The RB refactorings do some obscure dance around an errorBlock, which is always nil, as far as I can see. As I was trying to supply a non-nil error block, I was astonished to find out that even if it isn't nil, it won't be executed. Here's the relevant snippets:

checkPreconditions        | conditions block |        conditions := self preconditions.        conditions check                ifFalse:                        [block := conditions errorBlock.                        block notNil                                ifTrue: [self refactoringError: conditions errorString with: block]                                ifFalse: [self refactoringError: conditions errorString]]

refactoringError: aString with: aBlock        ^ RefactoringError signal: aString with: aBlock

I can't imagine how that could help anyone. I suggest either removing the errorBlock, or turning it into a good old instance variable, and running it in case of an error.

Cheers,

Niko

-- http://scg.unibe.ch/staff/Schwarz twitter.com/nes1983 Tel: +41 076 235 8683

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

-- Lukas Renggli www.lukas-renggli.ch

-- Lukas Renggli www.lukas-renggli.ch

The tool that triggers the refactoring, e.g. ORCommand>>#handleError: Ok, the code is quite ugly, but this is how the original refactoring browser implemented it. On 4 October 2010 16:06, Niko Schwarz <niko.schwarz@googlemail.com> wrote:

Ok, help me out, how is that block ever invoked? The above code snippet just appends the block as a parameter to an exception, which looks just funny.

Niko

On Mon, Oct 4, 2010 at 3:44 PM, Lukas Renggli <renggli@gmail.com> wrote:

...

See for senders of #errorBlock:, e.g. PushDownInstanceVariableRefactoring, RemoveClassRefactoring or RemoveInstanceRefactoring.

On 4 October 2010 15:40, Lukas Renggli <renggli@gmail.com> wrote:

...

This is used to validate the preconditions of refactorings and inform the user about any mismatches.

It should better not be removed.

Lukas

On 4 October 2010 15:36, Niko Schwarz <niko.schwarz@googlemail.com> wrote:

...

Hi,

The RB refactorings do some obscure dance around an errorBlock, which is always nil, as far as I can see. As I was trying to supply a non-nil error block, I was astonished to find out that even if it isn't nil, it won't be executed. Here's the relevant snippets:

checkPreconditions        | conditions block |        conditions := self preconditions.        conditions check                ifFalse:                        [block := conditions errorBlock.                        block notNil                                ifTrue: [self refactoringError: conditions errorString with: block]                                ifFalse: [self refactoringError: conditions errorString]]

refactoringError: aString with: aBlock        ^ RefactoringError signal: aString with: aBlock

I can't imagine how that could help anyone. I suggest either removing the errorBlock, or turning it into a good old instance variable, and running it in case of an error.

Cheers,

Niko

-- http://scg.unibe.ch/staff/Schwarz twitter.com/nes1983 Tel: +41 076 235 8683

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

-- Lukas Renggli www.lukas-renggli.ch

-- Lukas Renggli www.lukas-renggli.ch

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

-- http://scg.unibe.ch/staff/Schwarz twitter.com/nes1983 Tel: +41 076 235 8683

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

-- Lukas Renggli www.lukas-renggli.ch

Ok, let's summarize: If an error occurs, the handler block is not evaluated, but attached to a new exception, that exception is raised. Then, the caller catches that exception, retrieves the handler block, does some more ballyhoo, and then runs the handler block.

Yes, the block mostly contains some code to that allows the user to fix the problem (like to show all references of a class to be removed, etc.).

And you get invited to this construction by these methods in RBAbstractCondition:

errorBlock        ^self errorBlockFor: false

errorBlockFor: aBoolean        ^nil

These methods (and subclass implementations) are used to retrieve the right block that caused the composite condition to fail.

We can clean it up on the next sprint, if you want :)

The implementation in RBCondition is fine, the problem is more the way how exceptions are used to pass the values. This mainly stems from the fact that exceptions evolved over the past 15 years and are no longer the same as the signals that were used when the refactoring code was originally written. Now of course it is difficult to change this code because there are various users out there that depend on it, e.g. OB, RefactoringBrowser, SmaCC, ... Lukas -- Lukas Renggli www.lukas-renggli.ch