> On 5 Jun 2024, at 10:22, Tim Mackinnon <tim@testit.works> wrote:
>
> Hi Stef - I saw that blog post - which looks like its essentially the help
> text that comes up in the rewrite tool when you press the help button. I was
> going to submit to add to the help text as I think a better example that
> pulls all the ideas together is helpful. However, I have realised the Rewrite
> tool is not the main pharo-project repo and comes under NewTools? How do I
> find out where that repo is to submit a pr?
>
> I think the help button on that tool should launch the normal doc browser
> with the text in the normal help format vs a seperate window? The help
> browser could probably use some work but at least if help is one place it all
> will get updated together?
Yes it can be nice to have a single documentation so that when we improve it
will benefit directly
to the tools.
>
> I was going to propose the following example which helped me a lot - if I
> wanted to rewrite how logging was done in the following example (and separate
> the Transcript line into 2 lines):
>
> appLog: anObject
> "Log anObject for debugging"
> | date |
> date := Date today.
> Transcript traceCr: 'debug: ', date printString, anObject printString.
> date isLeapYear ifTrue: [ ^nil ].
> ^anObject
>
> I could write a match expression like this:
>
> appLog: `arg
> | `@temps |
> `.@before.
> Transcript traceCr: `#str , `@exp1, `@exp2.
> `.@after
>
> and a rewrite rule like this:
>
> appLog: `arg
> | `@temps |
> `.@before.
> Transcript cr; traceCr: `#str, `@exp1.
> `arg traceCr.
> `.@after
>
> I think this demonstrates most of the concepts that caught me out - namely:
> you need to call out the variable matching explicitly -= the .@before (list
> statement matching doesn't match variables - and using @temps will match 0 or
> more vars, so its safer to include it if matching expressions that may have
> any otherwise nothing will match.
>
> If you want to match a string you need to match it as a literal e.g. `#str.
>
> You may need to match 0 or more statements before and after your target
> expression hence `.@before and `.@after but its important to include the
> trailing "." on the @before. to cut off before the expression you are trying
> to match next.
>
> I didn't fully understand in my example is why I can't match the remaining
> statements after the transcript string using just `.@exp1. I seem to have to
> call out both expressions which I now realise (and worth documenting) is
> because the AST for that line is actually VariableNode(transcript) +
> MsgNode(MsgNode(Literal+MsgNode(exp1)), MsgNode(exp2) and so you have to call
> them all out to match things or match the entire sequence with @stmnt (in my
> case I wanted to pick them apart). I think the top tip in all of this is to
> use the handy source code ast inspector to help you reason on the structure.
>
Sometimes this is arcane to me.
I think that I would have to concentrate and do a full review of code using
tons of new tests :)
> The above is probably the gist of the kinds of docs we would need, and happy
> to submit a pr with this in it.
please do.
>
> On Wed, 5 Jun 2024, at 6:35 AM, stephane ducasse wrote:
>> Hi Tim
>>
>> This is on my todo to have a serious look at the pattern matcher
>> Now one of the few documentation that we have in the
>> blog and I ported it in the blog post in /doc folder.
>>
>> S
>>
>>
>>
>>
>>> On 4 Jun 2024, at 01:35, Tim Mackinnon <tim@testit.works> wrote:
>>>
>>> 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
>>
>>
>> Stéphane Ducasse
>> http://stephane.ducasse.free.fr
>> 06 30 93 66 73
>>
>> "If you knew today was your last day on earth, what would you do
>> differently? ....ESPECIALLY if, by doing something different, today might
>> not be your last day on earth.” Calvin & Hobbes
Stéphane Ducasse
http://stephane.ducasse.free.fr
06 30 93 66 73
"If you knew today was your last day on earth, what would you do differently?
....ESPECIALLY if, by doing something different, today might not be your last
day on earth.” Calvin & Hobbes
