Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push up #removeFromTree from MessageNode to ProgramNode #17925

Merged
merged 1 commit into from
Mar 5, 2025

Conversation

jecisc
Copy link
Member

@jecisc jecisc commented Mar 3, 2025

In multiple projects I'm doing AST manipulation (fro example in Famix or in my project Chanel) and I need to remove some nodes from an AST. Currently it's possible only on MessageNodes but this should be possible on much more nodes. I propose to push up this method on ProgramNodes

In multiple projects I'm doing AST manipulation (fro example in Famix or in my project Chanel) and I need to remove some nodes from an AST. Currently it's possible only on MessageNodes but this should be possible on much more nodes. 
I propose to push up this method on ProgramNodes
@Ducasse
Copy link
Member

Ducasse commented Mar 3, 2025

Do you know if the parent children are updated?
I mean if removing does not break the parent children invariant (referencing each other)?

@MarcusDenker
Copy link
Member

Failing tests on win not related:

windows-64 / Tests-windows-64 / Windows64.Traits.Tests.TraitFileOutTest.testFileOutTrait 0.27 sec 1
windows-64 / Tests-windows-64 / Windows64.Traits.Tests.TraitFileOutTest.testRecompiling 8.4 sec 1
windows-64 / Tests-windows-64 / Windows64.Traits.Tests.TraitFileOutTest.testRemovingMethods

@jecisc
Copy link
Member Author

jecisc commented Mar 4, 2025

Do you know if the parent children are updated? I mean if removing does not break the parent children invariant (referencing each other)?

Yes, using this method will either remove the node from the parent or replace it by its receiver in other cases

@Ducasse Ducasse merged commit 23c0a8a into pharo-project:Pharo13 Mar 5, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants