[WIP] 12082 finder UI should be rewritten using spec2 #12403
Conversation
Why: Making the first step to rewrite Finder in Spec2. How: Creates a new class named FinderPresenter, which is a subclass of SpPresenter. The class provides a first layout for the new Finder. Internally it uses the same search environment in form of an RBBrowserEnvironment object as the old Finder. Tags: Finder, Spec2
Why: The old model for the Finder tool is was not extensible and encapsulates multiple responsibilities in one class. The new FinderModel class is much simpler and no longer implements the searches itself. How: Creates a new class named FinderModel, which is a subclass of Model. The class keeps track of the search environment and executes searches. Tags: Finder, Model
Why: Extracting searches into their own classes makes for a cleaner interface and responsibility separation. The benefit is a much more extensible FinderModel. How: Creates a new class named FinderSearch. This class defines an interface for searches in the Finder tool. All Finder searches must subclass this class. Tags: Finder, Search
Why: Implementing the selector search for the new Finder tool. How: Creates a new class named FinderSelectorSearch. This class implements the selector search of the old Finder tool as a subclass of FinderSearch. Tags: Finder, Search
Why: Implementing the class search for the new Finder tool How: Creates a new class named FinderClassSearch. This class implements the class search of Finder tool by subclassing FinderSearch Tags: Finder, Search
Why: Provide first list of default searches for the new Finder tool. How: Adds the class method #defaultSearches to FinderModel. This method provides a list of FinderSearch subclasses, which are used by the FinderModel for searching; in this case the previously implemented FinderSelectorSearch and FinderClassSearch. Tags: Finder, Model, Search
Why: Provide a way to add and remove searches from a FinderModel object. How: #addSearch: simply adds a FinderSearch object to the list of availableSearches in FinderModel. #removeSearchByName: removes all elements form the list of availableSearches whose name is eqal to the string given to the method. Tags: Finder, Model
Why: SearchStarted is the first of a few announcements that will made by the new Finder tool to announce important events, like a start or end of a search. How: Creates a new class named SearchStarted, which is a subclass of Announcement. Tags: Finder, Announcement
Why: SearchEnded is an announcements that will made by the new Finder tool to announce that a search has been finished. How: Creates a new class named SearchEnded, which is a subclass of Announcement. SearchEnded also provides the results of search. Tags: Finder, Announcement
Why: SearchTypesChanged is an announcement that will be made by the new Finder tool to announce a change in the searches made available by it. The new Finder tool allows to add and remove searches to it. This announcement signals a change to this list. How: Creates a new class named SearchTypesChanged, which is a subclass of the ValueChanged Announcement. It provides access to the new and old value. Tags: Finder, Announcement
Why: The classes were renamed to better avoid future name conflicts. How: Renames SearchStarted to FinderSearchStarted, SearchEnded to FinderSearchEnded and SearchTypesChanged to FinderSearchTypesChanged. Tags: Finder, Announcement
Why: Announcements were not implemented in the FinderModel yet. Therefor FinderModel did not announce anything and no way for users of it to listen for certain kind of announcements. How: Adds the methods searchStarted, searchEnded, searchTypesChanged, whenSearchStarted, whenSearchEnded and whenSearchTypesChanged to FinderModel. Tags: Finder, Model, Announcement
Why: Uses FinderModel in FinderPresenter and connects the widgets to it. Now searches can be performed via FinderPresenter and their results are displayed in a simple way. How: FinderPresenter is now a subclass of FinderPresenterWithModel. This change implements updateResults and updateResultsWith: methods, which when called update the resultTree with new data. updateSearchModes respectively updates the searchModeDropList with the models availableSearches. In subscribeToAnnouncements these update functions are used to called when the corresponding announcements are made by the model. Additionally the statusBar was disabled and left for future changes. connectPresenters and searchInPackages got adjusted to call the right methods on the model. Tags: Finder, Model, Presenter, Spec2
Why: This method should have already been committed in the last change, but was missed. How: Please refer to the previous commit (852083c) for further information. Tags: Finder, Model, Presenter, Spec2
Why: A Dictionary was not particularly suited to be used as a result set as it did not provide a uniform way to treat its contents and the SpTreePresenter is not made for displaying a Dictionary. Thus FinderResult is implemented, which follows the composite pattern to represent different results in a uniform way and nesting of FinderResult objects in a tree like manner. How: FinderResult is the root composit class for results in the Finder tool. It implements a simple tree like structure by having a parent and children. Additionally it implements the capabilities for actions and the actions for its content itsef. These actions correspond to the result actions of the Finder tool. Tags: Finder, Result
Why: FinderResult is the root composite object for results in the Finder tool. It provides a uniform way to use results in the tool, but does not implement any specific actions for a certian kind of result. FinderSelectorResult does this for selector results. How: FinderSelectorsResult subclasses FinderResult provides capabilities and action for selectors. Tags: Finder, Result
Why: Similar to FinderSelectorResult this class subclasses FinderResult to implement capabilities and actions for classes. Tags: Finder, Result
…composite object. Why: Use the FinderResult composite objects as return value to simplify results in the Finder tool. Tags: Finder, Search
…posite object. Why: Use the FinderResult composite objects as return value to simplify results in the Finder tool. Tags: Finder, Search
Why: FinderSearch still mentioned search results in form of a Dictionary instead of a FinderResult. Tags: Finder, Search
Why: results is a much simpler more fitting term, especially now that FinderSearch no longer return results in form of a Dictionary, but as FinderResult composite objects. How: Rename resultDictionary instance variable and its accessors to results. Tag: Finder, Model
Tags: Finder, Presenter
Why: For a better user experience the result buttons now take the capabilities of the FinderResult's into account and are enabled/disabled accordingly. How: Disable resultButtons by default. As soon as an item is selected in the resultTree widget, query its capabilities and enable/disable the result buttons accordingly. Tags: Finder, Presenter
Why: This should have been in the last commit. How: This change calls updateResultButtonsFor: every time a result is selected in the resultTree widget. Tags: Finder, Presenter
Why: Sets values regarding the window for the FinderPresenter, like title and extent. Tags: Finder, Presenter
Why: The FinderPresenter class is getting very complex and big. By extracting the result buttons into their own widget it becomes more manageable again. How: Extracting the result buttons into their own class FinderResultButtonBar.
Why: This was missed in the last commit. Tags: Finder, Presenter
Why: This removes code, that is no longer used in the class. Tags: Finder, Presenter
Why: To reduce the complexity of the FinderPresenter class, all search bar related widgets are extracted into the new FinderSearchBar class. Tags: Finder, Presenter
|
Thanks this is cool! Could you rename FinderPresenter into StFinderPresenter because we are trying to have all the tools starting with St. I'm sure that Hernan @hernanmd will have a look :). Monday I hope to get some free time to have a look. |
|
I'm thinking how we could share changes and progress. I do not know if we are able to push to your PR. |
Of course I can rename the class. Will do that for the next PR. I assume that the naming then applies to the support classes too. I forgot to mention a bug, which I can't seem to pinpoint. but the first search performed doesn't seem to be displayed by the SpTreePresenter. The nesting just doesn't show up. All further searches work though.
I don't quite know what you mean. Should I create a new git repository? How would this work. I am not that familiar with git workflows yet. |
|
About the support classes we do not have a convention. The convention I know is for Presenter. About the workflow. |
|
So I created a new repository for the development of NewTools-Finder. |
I think there is a typo in your baseline: method. baseline: spec
<baseline>
spec for: #common do: [
spec
package: #'NewTool-Finder';
package: #'NewTool-Finder-Tests' with: [ spec requires: #( #'NewTool-Finder' ) ] ]According to your repository it should be NewTools-Finder instead of NewTool. |
Thanks for reporting. Fixed the typo. I also added the two missing finder widget classes which I forgot to add yesterday. |
|
Thanks I feel bad not having more cycle for this. |
|
Don't worry. I am not able to work as much on it as I want to too. This weekend I am probably not going to have any time do expand on this, but we will get there. |
|
This is a super job @n1toxyz !! Would you mind adding in the description of the PR a bullet list of things that are required to move this PR from WIP to ready state? That would give some visibility on its readiness, and maybe other community members could jump in to help |
|
Hi @guillep, that is a good idea. I added a first todo list to the README of the project. I will list it here nonetheless. Contributions are welcome. The tool is already usable, but does not implement all kinds of searches of the old Finder tool yet.
|
|
Nice work :) And when it will be ready, I think it will be better to integrate this to NewTools instead of Pharo |
|
Yes once ready we should mobe it to NewTools |
|
I can help you reviewing it if you like |
|
Thanks for looking at it. I am happy for any feedback. I didn't have much time the last weeks and will be busy this weekend too, but hope to get a lot more done afterwards. However I noticed yesterday when implementing the Pragma search, that it was more or less broken in the current Finder as some buttons result in exceptions when clicked und others not performing the correct action. At the same time I find the current layout and the layout of my rework a bit slow in regards to the UI/UX and I myself am using mostly the Spotter to find stuff. I think however that the Finder has some advantages/potential which the Spotter is not going to fullfill, like displaying/finding results related to each other. Thats why I have been looking around at other Finder tools in other IDEs for some ideas, but ultimately I want to do a little survey in the mailinglist to get an idea how or if people are using the Finder, what features do they like and what gets on their nerves and most importantly what are they missing? Otherwise, I think, we are wasting potential. I can for example think about adding an additional search for repositories and commits, which lets you easily find changes or a simple calculator by evaluating math expressions in the search bar. I cant to get rid of the button bar at the bottom and have these options displayed in a context menu on right clicking the desired result. Additional the standard keybindings should also work as expected (Ctrl+b for browse, etc...). But I am going to stop rambling for now. I have some ideas but I have to bring them to paper first in a structured fashion and maybe do some wireframing. |
|
:) I would say, make it, release it, iterate :) |
|
It is good that you are thinking on how to improve the Finder! Like Stéphane said, I will also say that to first make it work and then we can iterate. When you will have the time, moving the code to a separated repo and maybe documented a little will help us review it and help with the things that are not working as expected |
|
I could not load the baseline (but I do not get why it is not working) so I loaded the packages manually |
|
When I do I get a DNU |
|
When I load the tests package it is empty and I do not know if this is normal. |
|
I will do some PRs. |
|
This is the way to open it |
|
Oops it looks like I pushed directly the repo. Sorry about it. |
Yes, I will focus for now on shipping a Spec2 version of the old Finder with its bugs fixed. That way it can be merged and is already available.
Sadly I don't understand baselines myself yet, but yes it needs fixing.
Yes, it is empty for now. I just created it for future tests, but have not implemented any yet. In some cases @Ducasse Thanks for your work. I merged the changes. I planned from the beginning to eventually implement a class named StFinder which subclasses SpApplication and which will provide the tools to the presenter and model, but haven't done it yet as I still lack a bit of understanding how to correctly use SpApplication and wanted to focus on the core functionality first.
This is the last missing piece. Afterwards it is mostly wrapping things up, polishing and testing, but all the functionality of the old Finder should be implemented by then. |
|
I have the impression that we should just open stFinder via the StApplication. |
|
The application holds things like:
It should not be hard to use it for a new tool |
|
Now we should show the result in a text pane too. We should probably use a SpCodePresenter with beForMethod: |
I didn't know about that. But makes sense when the other tools are started from there too. The result pane is missing, you are right. I thought we can implement this as a vertical split next to the result tree, instead of opening below the buttons at the bottom, but this might be to small for a code listing. I will test this. |
|
Has moved to the NewTools repository. |
This my first start for rewriting the Finder tool in Spec2.
At first I wanted to just port the FinderUI class to Spec2, but noticed that the old Finder model didn't seem to fit into
the Spec2 programming style. It is imho complicated and not very extensible.
So I began to create a new model and separate the searches into their own classes as well. With the new model you can even add new searches to a running Finder instance and creating a new search is done by simply subclassing FinderSearch and implementing 3 methods.
Additional I use Announcements to notify FinderPresenter of changes from the model. This is important as the search is
done in its own subprocess.
I am still not quite done and not all searches are implemented yet. However, I am interested in feedback. Is this so far to your liking or do you think I am going in the wrong direction? Please let me know as I am still new to Spec2 und
contributing :)
The old Finder is not changed or deleted yet. I plan to replace the Finder class with one of the same name but as a subclass of SpApplication.
You can try the new Finder by running the following code, once loaded:
(FinderPresenter on: FinderModel new) open