Spring Tool Suite
  1. Spring Tool Suite
  2. STS-1419

User should be warned if any Grails-related commands are executed, but no Grails installs are configured

    Details

    • Type: Improvement Improvement
    • Status: Resolved Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5.1.RELEASE
    • Fix Version/s: 2.9.0.M1
    • Component/s: GRAILS
    • Labels:
      None

      Description

      In STS-1413, there is a situation where a user did not realize that there were no grails installs in the workspace and spent some time trying to figure out why. STS should be smarter about this and let the user know that there are no installs around (and ask them to configure one).

      This already happens for the new Grails Project wizard, but could happen in more places.

        Activity

        Hide
        Andy Clement (c) added a comment -

        What makes it particularly cryptic is that once in this situation, with a broken imported project and no grails defined, you don't even see the errors that would tell you whats wrong. If in that situation you try and run a refresh dependencies (for example), there is an error log entry that tells you exactly the issue:

        java.lang.IllegalArgumentException: Could not find a grails install.  Please configure an install from the Grails preferences page.
        at com.springsource.sts.grails.commands.GrailsCommandUtils.deleteOutOfSynchPlugins(GrailsCommandUtils.java:382)
        at com.springsource.sts.grails.commands.GrailsCommandUtils.refreshDependencies(GrailsCommandUtils.java:313)
        at com.springsource.sts.grails.core.internal.classpath.GrailsClasspathContainerUpdateJob.runInWorkspace(GrailsClasspathContainerUpdateJob.java:74)
        at org.eclipse.core.internal.resources.InternalWorkspaceJob.run(InternalWorkspaceJob.java:38)
        at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
        

        but if you don't have the error log open, you wouldn't know what happened - there is no indication in the UI that anything succeeded or failed.

        This already happens for the new Grails Project wizard, but could happen in more places.

        Yes, I guess any activity that requires a grails install (to run something or even just query something) should trigger the definition dialog. Hopefully most actions are routed through a few command points so the logic to handle this kind of situation won't have to be scattered everywhere.

        Show
        Andy Clement (c) added a comment - What makes it particularly cryptic is that once in this situation, with a broken imported project and no grails defined, you don't even see the errors that would tell you whats wrong. If in that situation you try and run a refresh dependencies (for example), there is an error log entry that tells you exactly the issue: java.lang.IllegalArgumentException: Could not find a grails install. Please configure an install from the Grails preferences page. at com.springsource.sts.grails.commands.GrailsCommandUtils.deleteOutOfSynchPlugins(GrailsCommandUtils.java:382) at com.springsource.sts.grails.commands.GrailsCommandUtils.refreshDependencies(GrailsCommandUtils.java:313) at com.springsource.sts.grails.core.internal.classpath.GrailsClasspathContainerUpdateJob.runInWorkspace(GrailsClasspathContainerUpdateJob.java:74) at org.eclipse.core.internal.resources.InternalWorkspaceJob.run(InternalWorkspaceJob.java:38) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) but if you don't have the error log open, you wouldn't know what happened - there is no indication in the UI that anything succeeded or failed. This already happens for the new Grails Project wizard, but could happen in more places. Yes, I guess any activity that requires a grails install (to run something or even just query something) should trigger the definition dialog. Hopefully most actions are routed through a few command points so the logic to handle this kind of situation won't have to be scattered everywhere.
        Hide
        Kris De Volder (c) added a comment -

        One slight complication with those "few command points" is that they aren't in UI code. Anyhow, it seems like most closely related to stuff I did on command execution so I'll assign this to me.

        Show
        Kris De Volder (c) added a comment - One slight complication with those "few command points" is that they aren't in UI code. Anyhow, it seems like most closely related to stuff I did on command execution so I'll assign this to me.
        Hide
        Andrew Eisenberg (c) added a comment - - edited

        In Eclipse, the common way around getting UI hooks into non-UI code is to provide an adapter using the Adapter framework (see the org.eclipse.core.runtime.adapters extension point). It's a little bit ugly, but it ensures that the UI/Core separations remains.

        Show
        Andrew Eisenberg (c) added a comment - - edited In Eclipse, the common way around getting UI hooks into non-UI code is to provide an adapter using the Adapter framework (see the org.eclipse.core.runtime.adapters extension point). It's a little bit ugly, but it ensures that the UI/Core separations remains.
        Hide
        Kris De Volder (c) added a comment -

        I followed this particular exception from its source, and found that it didn't get reported in the usual way (dialog box) because the job that has the error (ClassPathContainerUpdateJob) decided to log the error and return a "cancel status" rather than notify job manager of the problem by returning an error status.

        I think in general we are trying to raise appropriate exceptions when something/anything goes wrong in executing grails commands. But we also need the contexts in which those Exeptions occur to handle them appropriately.

        In this case, I think the job in the past tended to generate too many spurious errors, which is why its error were being "silently" muffled away. But with the better handling of the sequencing of stuff that happens after grails commands get executed, this job now doesn't seem to cause spurrious errors anymore (errors tend to be real problems now), so I've changed it to propagate the error to the Job scheduler, which in turn will show a dialog to the user.

        I've tested it a bit and it looks stable. The error dialog looks reasonable and seems to appear at the right times (e.g. when you import a grails project and there's no Grails install, or when you try to run Refresh dependecies manually and there is not Grails install. So I'm going to commit it shortly.

        Show
        Kris De Volder (c) added a comment - I followed this particular exception from its source, and found that it didn't get reported in the usual way (dialog box) because the job that has the error (ClassPathContainerUpdateJob) decided to log the error and return a "cancel status" rather than notify job manager of the problem by returning an error status. I think in general we are trying to raise appropriate exceptions when something/anything goes wrong in executing grails commands. But we also need the contexts in which those Exeptions occur to handle them appropriately. In this case, I think the job in the past tended to generate too many spurious errors, which is why its error were being "silently" muffled away. But with the better handling of the sequencing of stuff that happens after grails commands get executed, this job now doesn't seem to cause spurrious errors anymore (errors tend to be real problems now), so I've changed it to propagate the error to the Job scheduler, which in turn will show a dialog to the user. I've tested it a bit and it looks stable. The error dialog looks reasonable and seems to appear at the right times (e.g. when you import a grails project and there's no Grails install, or when you try to run Refresh dependecies manually and there is not Grails install. So I'm going to commit it shortly.
        Hide
        Kris De Volder (c) added a comment -

        Resolving this issue.

        The specific case mentioned in this issue should be fixed now. If there are more cases where errors should be made more visible to users, I think it is best to open another issue for them.

        Show
        Kris De Volder (c) added a comment - Resolving this issue. The specific case mentioned in this issue should be fixed now. If there are more cases where errors should be made more visible to users, I think it is best to open another issue for them.

          People

          • Assignee:
            Kris De Volder (c)
            Reporter:
            Andrew Eisenberg (c)
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              First Response Date: