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

Add subclass driver #16353

Merged
merged 18 commits into from
May 31, 2024
Merged

Add subclass driver #16353

merged 18 commits into from
May 31, 2024

Conversation

hernanmd
Copy link
Contributor

This PR implements a first version of "Add subclass" refactoring as a driver:

add_new_subclass_edit.mp4

Main features

  • The Spec UI to add a new subclass (see video above).
  • It checks and informs interactively if the class already exists.
  • It checks invalid class names.
  • Default class comment could be customized implementing blankCommentTemplate in the class selected as superclass.

Implementation notes

  • A RBAddNewClassRefactoring which differs from the Insert New Class refactoring in that does not reparent the subclasses to the new class.
  • The driver implementation in ReAddSubclassDriver and ReAddSubclassDriverTest.

Known issues

  • CmdCommandAborted signal and CmCommandAborted signal are failing right now and it should be investigated why (debugger keep opening which could mean the exception is not properly handled).
  • Next version will include more tests.

Copy link
Member

@balsa-sarenac balsa-sarenac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I really like the UI for adding new subclass! Left some comments inline, based on our discussion

@estebanlm estebanlm changed the base branch from Pharo12 to Pharo13 April 26, 2024 09:11
Hernán Morales Durand added 9 commits May 23, 2024 15:23
Add tests
Migrate the Syc user

pick e5f0165 Add RBAddNewClassRefactoring and ReAddSubclassDriver Add tests Migrate the Syc user
pick 6e9c5a4 Add StRefactoringAddClassPresenter Update #SycCmAddSubclassCommand
pick a763b30 Update privateTransform so we don't reparent classes in add subclass. Add event hooks in add class presenter.
Update #SycCmAddSubclassCommand
Enhance Add subclass UI
…m the driver.

Fix the layout so comment presenter is bigger.
Add use comment template checkbox.
Refactoring initialize presenters.
Show superclass in title dialog.
Fix updating tag items.
Temporary fix of onCancel: block using CmCommandAborted or CmdCommandAborted, both raising a Debugger when signaled.
Restore previous versions.

pick 9b61541 Remove classTraits : '{} + TraitedClass'
pick 3d53473 Remove commits from other branch accidentally included. Restore previous versions.
pick 5422119 Clean another leftover from another branch
Hernán Morales Durand added 2 commits May 23, 2024 15:30
…RBInsertNewClassRefactoring

Remove unnecessary line in ReAddSubclassDriver>>configureRefactoring
Remove unnecessary instance variable newClassPresenter in StRefactoringAddClassPresenter. Re-use 'textInput' from superclass.
Update initialExtent setting.
Remove unnecessary #example method.
Hernán Morales Durand added 3 commits May 27, 2024 16:06
Provide two instance creation helpers to provide a dialog with the superclass and its packages.
Refactor ReAddSubclassDriver methods.
@MarcusDenker
Copy link
Member

Two tests failing due to:

  • the following classes have unused instance variables and should be cleaned: SycCmAddSubclassCommand
  • the following classes have uncategorized methods: an OrderedCollection(RBInteractionDriver)

@Ducasse
Copy link
Member

Ducasse commented May 31, 2024

@hernanmd can you take the point of balsa into account so that we can integrate this issue.

hernanmd and others added 2 commits May 31, 2024 11:49
- the following classes have unused instance variables and should be cleaned: SycCmAddSubclassCommand
- the following classes have uncategorized methods: an OrderedCollection(RBInteractionDriver)
Re-use RBInsertNewClassRefactoring also for adding new refactoring
@hernanmd
Copy link
Contributor Author

@hernanmd can you take the point of balsa into account so that we can integrate this issue.

Done

@jecisc jecisc merged commit 020db83 into pharo-project:Pharo13 May 31, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants