This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Single file action such as Run File or Debug File (i.e. those implemented by o.n.m.prj.ui.actions.FileCommandAction) are controlled by the project type and are only enabled and supported for files that are known to the project type. For some files single file actions behavior could be implemented independently of the project type, but there is no way to plug into the same UI. Use Case 1: JavaScript provides a Run action to execute a .js file. The action is currently exposed in editor popup menu ("Run"), but it would be more intuitive to integrate it into the Run File action. Currently Run File action is disabled for .js files. (Issue 102040) Use Case 2: Ant file has a Run Target action and Debug Target action but Run File and Debug File are disabled. They could be enabled and Run/Debug the default target in file.
Proposed solution is to provide an interface FileCommandActionProvider similar to o.n.spi.project.ActionProvider that would be implemented by modules that want to provide single action semantics and placed into default lookup. public interface FileCommandActionProvider { // Expected only COMMAND_..._SINGLE String[] getSupportedActions(); void invokeAction(String command, Lookup context) throws IAE; boolean isActionEnabled(String command, Lookup context) throws IAE; } Then the FileActionCommand would be modified. In case the project type ActionProvider does not enable the given action for a given context FileActionCommand would look for FileCommandActionProvider instances to check if any of them enables the action for given context.
Created attachment 41409 [details] FileCommandActionProvider.java
Created attachment 41410 [details] modified FileCommandaction
Created attachment 41411 [details] diff of FileCommandAction
Asking for API review and comments.
[JG01] Why invent another interface? IMHO we could just look for ActionProvider in default lookup. [JG02] Do not use HashSet. Preserve order. [JG03] Do not log anything at INFO. [JG04] The code for (String action : p.getSupportedActions()) { if (action.equals(getCommand())) { providers.add(p); } } could be simplified to if (Arrays.asList(p.getSupportedActions()).contains(command)) { providers.add(p); } [JG05] The actionPerformed method is wrong since it could perform an action multiple times. After any call to invokeAction, return from the method. BTW (as I mentioned to Honza Pokorsky) the new file FileCommandActionProvider.java has the wrong license header. For now you have to customize cddl-license.txt in every userdir.
Jesse, thanks for API/code review. I agree with all your comments: JG01 - I will just add javadoc comment into ActionProvider and not add FCAP. JG02 - changed to ArrayList JG03 - The logging was just for my testing, sorry I left it in there. JG04 - cool, done JG05 - thanks for catching that
Created attachment 41414 [details] updated diff w/Jesse's comments addressed
Created attachment 41415 [details] updated diff w/Jesse's comments addressed
MK01: I assume the actions provided in this way will not rely on build script or any internal for ant projects, correct? I think this is ought to work with maven based projects as well.
MK01 Yes, this is my assumption as well, at least for the cases I had in mind. I am not sure it has to be mentioned explicitly in javadoc, though.
Y01 I somehow feel the test coverage is not really big, could you spend a little bit of time and make it a bit greater? I guess it is one simple use of MockServices... Y02 I can see the new diff is much smaller, but still we need to increase the spec version of the module and have an apichange entry. That way people will be able to depend on the version of the module that does provide this new API.
In issue 102040 you mention that Run File is enabled for Java files, but disabled for JavaScript files. Thus I assume you are talking about files inside a project. Then an alternative solution could be a LookupMerger<ActionProvider> and registering your action provider in the project's lookup. That would have the advantage that your ActionProvider can be project-dependent. The disadvantage is that your ActionProvider would be project-dependent (so for JS files you need to register it in the lookups of all projects) and that it wouldn't work for files not owned by any project. The former could that be solved by a Projects/Lookup folder included in the lookups of all projects (which would be nice to have anyway), and the latter is larger (see e.g. issue 71231, which Pavel's API change does not address -- unless we agree that db can depend on projects/projectapi of course). Anyway, if we go on with the proposed change: AB01: I would say better not to call the default lookup ActionProvider's if project.length > 1; do it only when == 0. AB02: I guess you want the action for JS files in the main menu to be named "Run <file>", as it is for Java files. This doesn't seem to be allowed by the patch. AB03: FileCommandAction is used in the main menu, thus it can be long-lived, thus ActionProvider's can be added and removed to/from the default lookup during the action's lifetime, thus you probably need to listen on the default lookup for ActionProvider's. Haven't tried your patch, just guessing, so sorry if it's a false alarm.
"the advantage that your ActionProvider can be project-dependent" is probably no real advantage, since you can just register a global AP which inspects the project owning the file for various conditions and conditionally enables the action. AB03 is probably a real problem (for the global menu item, not for context menu items). Is it necessary to precompute the set of ActionProvider's? Is there really a performance issue with just asking Lookup each time?
Re. project-dependent ActionProvider's: I was thinking that e.g. registering an AP in the Projects/o-n-m-web-project/Lookup folder inherently enables that AP only in the context of a web project, and that actions really want to do that. But you are right, actions are probably sensitive to the services provided by the project, not to the project type (in the example above it would e.g. try to find a WebModule for the current file).
I do not have time to work on it now. When the comments are addressed it should be submitted for another review. BTW, one other use case might be Run SQL in .sql file.
I did not mean to task Milos with this, I just set it to default owner. If I have time I will reassign back to myself.
Obviously not in M10; status?
*** Issue 136835 has been marked as a duplicate of this issue. ***
This needs to be reconsidered and resubmitted. Miloši do you want to work on it, or should I?
I will take it.
Created attachment 84850 [details] Pavel's last patch, updated for current sources
Created attachment 84854 [details] Addresses AB01, AB02, AB03
Created attachment 84855 [details] Sample module project using new API
Attached a sample module which allows you to F9 a *.pdf by running evince on it. Seems to work fine. Currently it looks like there are going to be maybe three impls, all RUN_SINGLE on a single file of specific type. If there prove to be a lot of impls, it would be undesirable to iterate them all in global lookup (too many classes loaded). In that case it would be straightforward to create an annotation-based registration which lazily loads impls by command and file type, e.g. public class RunJavaScript { @GlobalFileSensitiveActionRegistration(command=ActionProvider.COMMAND_RUN_SINGLE, mimeType="text/x-javascript") public static void run(FileObject file) {...} } Right now that seems premature, so I would leave it to a future API request if the need arises.
Created attachment 84856 [details] New patch with test and API change information
Please review the last patch.
code looks ok, I'm a bit worried generally about the actual implementations though. I don't see how one can write an action provider without the ties to the project type, so my worry is that some impls might opt for ant based support only. But let's see..
Milos, if you look at the issues that are currently blocked by this (running Ruby, JavaScript and SQL), they are all pretty independent of Ant and I don't see why any of them would want to be tied to Ant. So I don't share your worries. This enhancement is really useful, and will help NetBeans become more competitive with lightweight text editors. Jesse, thanks many times for taking it on.
core-main #925e1deec0a8