Hey Gabriel - thanks for chipping in - you've made me realise that its perhaps 
easier to give a simpler example (for the multiple replacement)., and while 
trying to explain in simpler terms I managed to solve the problem myself - but 
it does demonstrate the docs around this need a bit more work as I didn't find 
it easy to reason on.

I am still confused what the setting  "this rule is for a method" does - so am 
keen for someone to shed light.

Anyway for my example it seems that I have to be very specific in how I match 
things for example I had to use the search pattern:

taskbarButtonMenu: `arg
  | `@temps |
  `.@before.

  moveSubmenu
    addToggle: 'Move right' translated
    target: self
    selector: #taskbarMoveRight
    getStateSelector: nil
    enablementSelector: #canBeMovedToRight.

  `.@after

And it took me ages to realise that I have to specify the temp vars matching, 
and also understand that the @before and @after both needed the "." to 
represent lists of expressions both before and after and then the really subtle 
 "." after @before which I guess prevents greedy matching.

With all of that in place,  could then have the write rule add an extra 
statement after the moveSubmenu (or as many as I want)

e.g.

taskbarButtonMenu: `arg
  | `@temps |
  `.@before.

  moveSubmenu
    addToggle: 'Move right' translated
    target: self
    selector: #taskbarMoveRight
    getStateSelector: nil
    enablementSelector: #canBeMovedToRight.

  moveSubmenu 
     addToggle: 'Move to...' translated 
    target: self 
    selector: #taskbarMoveTo 
    getStateSelector: nil.

  `.@after


I'm curious now whether there is a more efficient way to do what I have done? 
Possibly the {} syntax could help now i've figured out the basic matching.

I really find it easy to confuse that if you don't specify the @temps section 
that can leave out a lot of matches (it essentialy means 0 or more temps vs. 
leaving it out means no temps at all). This is a trap that the docs should call 
out better I think.

Anyway - now I just need to figure out how to do this programatically - and 
possibly I can write a few more useful refactorings I've also thought it was 
odd that were missing.

On Mon, 3 Jun 2024, at 12:19 PM, Gabriel Cotelli wrote:
> I've used the rewrite tool extensively but never tried to create more than 
> one expression from the original one. I think that this probably won't work 
> because in your example the code is just statements, but the matching 
> expression can be in code like this:
> 
> self doSomething: (
> moveSubmenu
>    addToggle: 'Move right' translated
>    target: self
>    selector: #taskbarMoveRight
>    getStateSelector: nil
>    enablementSelector: #canBeMovedToRight)
> 
> and in this case, you cannot replace this with two statements. So I'm tempted 
> to say that what you want is not supported.
> 
> Regards, Gabriel
> 
> On Mon, Jun 3, 2024 at 5:44 AM Tim Mackinnon <tim@testit.works> wrote:
>> __
>> I've never really learned the rewrite tool but while waiting for changes to 
>> propagate through P12, I thought maybe I could use it to safely patch 
>> changes until there are official updates (and it was a good chance to get 
>> more familiar with it).
>> 
>> I thought there was more documentation available for how it works 
>> (interested in any old articles - but the online help in P12 is ok - 
>> although it doesn't display in markdown - in fact give there is markdown 
>> support now - I am a bit confused what should appear more readable these 
>> days - and how to fix/contribute changes to help).
>> 
>> This aside - I wanted to patch the SystemWindow>>taskbarButtonMenu: (and add 
>> my fix to move tabs to any position).
>> 
>> I started writing the following search expression - the find the last call 
>> to #addToggle:... method call that has a literal referencing 
>> #canBeMovedRight - as I want to add my new menu item after it.
>> 
>> e.g.
>> 
>> moveSubmenu `@addToggle: `{ :node | node isLiteralNode and: [ node value = 
>> #canBeMovedToRight] } 
>> 
>> but I'm not sure my {} filter block is ever called, as the above matches 
>> both addToggle:... calls, and not the one I'm after? I don't understand this 
>> - any have ideas on the proper syntax for generic matching?
>> 
>> Anyway - I decided that in my case there is only one - so I can match it 
>> explicitly and so I  search on the full call:
>> 
>> and I entered this:
>> moveSubmenu
>>    addToggle: 'Move right' translated
>>    target: self
>>    selector: #taskbarMoveRight
>>    getStateSelector: nil
>>    enablementSelector: #canBeMovedToRight.
>> 
>> Which matches exactly - then I want to replace the single statement with 2 
>> statements e.g.
>> 
>> moveSubmenu
>>    addToggle: 'Move right' translated
>>    target: self
>>    selector: #taskbarMoveRight
>>    getStateSelector: nil
>>    enablementSelector: #canBeMovedToRight.
>> 
>> moveSubmenu
>>    addToggle: 'Move to' translated
>>    target: self
>>    selector: #taskbarMoveTo
>>    getStateSelector: nil.
>> 
>> 
>> However this gives an error in the rewrite tool (actually it results in a 
>> zero change, that then gives an error for EpMonitor).
>> 
>> But my question is - why can't you rewrite a single statement to multiple 
>> ones? I have noticed that in this case I can cascade the message send - and 
>> this then rewrites properly - so it looks like it wants to replace a single 
>> node with a single node.
>> 
>> If I couldn't cascade - It seems I would have to rewrite with something 
>> like: [ "put multiple expressions here" ] value.
>> 
>> This seems an odd choice - but am I missing something obvious? 
>> 
>> Do you have to do what I'm proposing in 2 steps - rewrite to a block (or 
>> some equivalent) and then a 2nd rewrite to simplify the expression  (like a 
>> refactor step) - so its more provably correct?
>> 
>> I'm just curious on this - as it seemed time to learn this properly?
>> 
>> Tim

Reply via email to