osate / osate2

Open Source AADL2 Tool Environment
http://osate.org
Eclipse Public License 2.0
39 stars 8 forks source link

Populating classifier info view should be more intuitive #1748

Closed lwrage closed 5 years ago

lwrage commented 5 years ago

Currently the classifier info view is filled automatically based on a classifier selected in the AADL navigator.

It would be better to fill it based on a user command:

Remove automatic view population from classifier selection in navigator and replace it with

AaronGreenhouse commented 5 years ago

This is actually harder than just reacting to the selection because you need to be able to

I am looking at org.eclipse.jdt.ui.actions.OpenTypeHierarchyAction to study the situation. Also relevant is org.eclipse.jdt.internal.ui.util.OpenTypeHierarchyUtil.

AaronGreenhouse commented 5 years ago

Will be able to get rid of the synchronize/link with view functionality.

AaronGreenhouse commented 5 years ago

Added action that appears in the AADL Navigator and the Outline views.

Doesn't do anything yet, just hooked in an action.

AaronGreenhouse commented 5 years ago

Trying to get an action into the context menu for the AADL editor.

Found this in org.osate.xtext.aadl2.ui

    <extension
        point="org.eclipse.ui.commands">
        <command
            description="Organize With Clauses"
            id="org.osate.xtext.aadl2.ui.organize.with"
            name="Organize With Clauses">
        </command>
    </extension>
    <extension point="org.eclipse.ui.menus">
         <menuContribution
         allPopups="false"
         locationURI="popup:#TextEditorContext">
        <command commandId="org.osate.xtext.aadl2.ui.organize.with"
         style="push">
         <visibleWhen checkEnabled="false">
               <reference
                     definitionId="org.osate.xtext.aadl2.Aadl2.Editor.opened">
               </reference>
         </visibleWhen>
        </command>
      </menuContribution> 
   </extension>
    <extension
        point="org.eclipse.ui.handlers">
        <handler 
            class="org.osate.xtext.aadl2.ui.handlers.OrganizeWithHandler"
            commandId="org.osate.xtext.aadl2.ui.organize.with">
        </handler>
    </extension>

Hoping it will work for me too.

AaronGreenhouse commented 5 years ago

Okay, I got the command into the editor pop up, but the enablement is needs to be dealt with, because obviously, the selected item (text) cannot be adapted to Classifier. Probably need to override isEnabled in the handler class, but I have a feeling it's not that straightforward.

Come back to this later, just wanted to get the command into the menu for now.

AaronGreenhouse commented 5 years ago

Update the formerly empty command handler so that it makes the classifier information view active (and opens it if needed). This is the first step to giving it a new input.

This is easy to do, but took a while to find out how:

AaronGreenhouse commented 5 years ago

NOTE: look atOrganizeWithHandler for ideas about how to deal with command activation from the text editor.

AaronGreenhouse commented 5 years ago

Updated OpenInClassifierInfoViewHandler to process the current selection.

Need to update the view to take the input now. Also get to rip out all the link to editor stuff and the local selection handler.

AaronGreenhouse commented 5 years ago

Hmm, my command seems to be active for nodes that aren't Classifier nodes. need to look into that later.

AaronGreenhouse commented 5 years ago

Updated the handler to update the classifier information view input.

This was not working reliably at first. The handler was not getting the most up to date selection form the Outline or navigator views. This was because I had failed to implemented ClassifierInfoView.setFocus() property. It had the default null implementation. It is expected to forward the focus to one of the view controls. Not doing so messed up the state of the workbench it seems. I have fixed this to forward the focus to the member tree of the classifier info view. Now things works okay.

Still to do

AaronGreenhouse commented 5 years ago

Should also add context menu to the ClassifierInfoView that allows refocusing on a selected classifier.

AaronGreenhouse commented 5 years ago
  • Fix the command activation so that it only activates for Classifiers. I thought I did this before but it's not working correctly.

Works correctly in the Outline view. But enables for non Classifier objects in the AADL Navigator. (Correctly disabled when more than classifier is selected.)

AaronGreenhouse commented 5 years ago
  • Fix the command activation so that it only activates for Classifiers. I thought I did this before but it's not working correctly.

Works correctly in the Outline view. But enables for non Classifier objects in the AADL Navigator. (Correctly disabled when more than classifier is selected.)

Fixed this. The problem was in the plugin.xml in the definition of the handler. Had to make it

    <extension
          point="org.eclipse.ui.handlers">
       <handler
             class="org.osate.ui.handlers.OpenInClassifierInfoViewHandler"
             commandId="org.osate.ui.showInClassifierInfoView">
          <enabledWhen>
             <with
                   variable="selection">
                <and>
                   <count
                         value="1">
                   </count>
                   <iterate>
                      <or>
                         <!-- For the AADL Navigator View -->
                         <adapt
                               type="org.osate.aadl2.Classifier">
                            <!-- true -->
                         </adapt>

                         <!-- For the Outline View -->
                         <adapt type="org.eclipse.xtext.ui.editor.outline.impl.EObjectNode">
                             <test
                                   property="org.osate.ui.superType"
                                   value="Classifier"
                                   forcePluginActivation="true"/>
                         </adapt>
                      </or>
                   </iterate>
                </and>
             </with>
          </enabledWhen>
       </handler>

In particular, I had to add the adapt clause around the test clause. The test for superType only works on EObjectNode objects. If the current object was not an EObjectNode it was evaulating to true. Not sure why it would do this, but that is the fact. Wrapping it with the adapt causes a fail if the object is not an EObjectNode, and then we test the EObjectNode object for the super type.

AaronGreenhouse commented 5 years ago

For the XText editor, I made it so that the command is active all the time. The text selection is converted to an EObject using SelectionHelper. The classifier is derived from the object by either trying to get the classifier from it, or by searching for a containing classifier.

AaronGreenhouse commented 5 years ago

To do:

AaronGreenhouse commented 5 years ago

Do we need a keyboard shortcut? if so, which one?

lwrage commented 5 years ago

I don't think we should look for the containing classifier. I'd prefer if this works only if the classifier name is selected. Otherwise it should bring up a dialog that tells the user to select a classifier name. No keyboard shortcut necessary.

AaronGreenhouse commented 5 years ago

I don't think we should look for the containing classifier. I'd prefer if this works only if the classifier name is selected. Otherwise it should bring up a dialog that tells the user to select a classifier name.

It seems like getting the classifierinfo view connection to the XText editor to work the way we want is not very easy. I've been using SelectionHelper.getEObjectFromSelection() which relies on the xtext EObjectOffsetHelper class. But this method already tries to be too fancy for our needs. I don't see a straightforward way to determine if the selection if of a name reference node.

AaronGreenhouse commented 5 years ago

Update the help text

lwrage commented 5 years ago

I just realized that the selection stuff should be doable: Get the classifier as it is done now, find the nodes for its name in the node model and check if exactly these nodes are selected.

AaronGreenhouse commented 5 years ago

I just realized that the selection stuff should be doable: Get the classifier as it is done now, find the nodes for its name in the node model and check if exactly these nodes are selected.

Played with this this morning. Still difficult to get what we want. The parse tree really doesn't want to give up this information in the that we need. Not sure it's worth pursuing any further.