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

CreateMainClassAndOpenItInEditor #398

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions resources/fileTemplates/internal/HaxeMainClass.hx.ft
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package ${PACKAGE_NAME};
class ${NAME} {
public static function main() {
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be invisible to user, only for project generation

Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.progress.Task;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.project.ProjectType;
import com.intellij.openapi.project.ProjectTypeService;
import com.intellij.openapi.projectRoots.Sdk;
import com.intellij.openapi.roots.ModifiableRootModel;
import com.intellij.openapi.roots.ModuleRootManager;
Expand All @@ -40,6 +42,7 @@
import com.intellij.openapi.vfs.VfsUtil;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.VirtualFileManager;
import com.intellij.plugins.haxe.config.sdk.HaxeSdkType;
import com.intellij.plugins.haxe.hxml.HXMLFileType;
import com.intellij.plugins.haxe.hxml.psi.HXMLClasspath;
import com.intellij.plugins.haxe.hxml.psi.HXMLLib;
Expand Down Expand Up @@ -579,13 +582,20 @@ private HaxeClasspath getProjectClasspath(@NotNull ProjectTracker tracker) {
@NotNull
private void syncProjectClasspath(@NotNull ProjectTracker tracker) {
HaxeDebugTimeLog timeLog = new HaxeDebugTimeLog("syncProjectClasspath");
timeLog.stamp("Start synchronizing project " + tracker.getProject().getName());
Project project = tracker.getProject();
timeLog.stamp("Start synchronizing project " + project.getName());

Sdk sdk = HaxelibSdkUtils.lookupSdk(project);
boolean isHaxeSDK = sdk.getSdkType().equals(HaxeSdkType.getInstance());

if (!isHaxeSDK) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check which fixes #397

return;
}

Sdk sdk = HaxelibSdkUtils.lookupSdk(tracker.getProject());
HaxelibLibraryCache libCache = tracker.getSdkManager().getLibraryCache(sdk);
HaxeClasspath currentProjectClasspath = HaxelibClasspathUtils.getProjectLibraryClasspath(
tracker.getProject());
List<String> currentLibraryNames = HaxelibClasspathUtils.getProjectLibraryNames(tracker.getProject(), true);
project);
List<String> currentLibraryNames = HaxelibClasspathUtils.getProjectLibraryNames(project, true);
HaxeClasspath haxelibClasspaths = libCache.getClasspathForHaxelibs(currentLibraryNames);

// Libraries that we want to remove are those specified as 'haxelib' entries and are
Expand Down
20 changes: 11 additions & 9 deletions src/common/com/intellij/plugins/haxe/ide/HaxeFileTemplateUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.intellij.ide.fileTemplates.FileTemplate;
import com.intellij.ide.fileTemplates.FileTemplateManager;
import com.intellij.ide.fileTemplates.FileTemplateUtil;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Condition;
import com.intellij.plugins.haxe.HaxeFileType;
import com.intellij.plugins.haxe.nmml.NMMLFileType;
Expand All @@ -39,28 +40,28 @@
public class HaxeFileTemplateUtil {
private final static String HAXE_TEMPLATE_PREFIX = "Haxe";

public static List<FileTemplate> getApplicableTemplates() {
public static List<FileTemplate> getApplicableTemplates(Project project) {
return getApplicableTemplates(new Condition<FileTemplate>() {
@Override
public boolean value(FileTemplate fileTemplate) {
return HaxeFileType.DEFAULT_EXTENSION.equals(fileTemplate.getExtension());
}
});
}, project);
}

public static List<FileTemplate> getNMMLTemplates() {
public static List<FileTemplate> getNMMLTemplates(Project project) {
return getApplicableTemplates(new Condition<FileTemplate>() {
@Override
public boolean value(FileTemplate fileTemplate) {
return NMMLFileType.DEFAULT_EXTENSION.equals(fileTemplate.getExtension());
}
});
}, project);
}

public static List<FileTemplate> getApplicableTemplates(Condition<FileTemplate> filter) {
public static List<FileTemplate> getApplicableTemplates(Condition<FileTemplate> filter, Project project) {
List<FileTemplate> applicableTemplates = new SmartList<FileTemplate>();
applicableTemplates.addAll(ContainerUtil.findAll(FileTemplateManager.getInstance().getInternalTemplates(), filter));
applicableTemplates.addAll(ContainerUtil.findAll(FileTemplateManager.getInstance().getAllTemplates(), filter));
applicableTemplates.addAll(ContainerUtil.findAll(FileTemplateManager.getInstance(project).getInternalTemplates(), filter));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FileTemplateManager.getInstance() is deprecated

That is why I changed it to
FileTemplateManager.getInstance(project).getInternalTemplates()

Copy link
Contributor

Choose a reason for hiding this comment

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

You are still building and testing against v14.1.6, right? Did you happen to locally pull and test against the v15 build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, does it works?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, Srikanth is reviewing it right now, and I'm going to pull it later. Then I'll kick off new builds for your and Elias' (Ilya's) changes and make sure they all build correctly. (The unit tests for v15 do not run correctly, but that's a JetBrains bug that they are working on. We'll re-enable running them after their fix.)

applicableTemplates.addAll(ContainerUtil.findAll(FileTemplateManager.getInstance(project).getAllTemplates(), filter));
return applicableTemplates;
}

Expand Down Expand Up @@ -88,11 +89,12 @@ else if ("Enum".equals(name)) {

public static PsiElement createClass(String className, String packageName, PsiDirectory directory, String templateName, @org.jetbrains.annotations.Nullable java.lang.ClassLoader classLoader)
throws Exception {
final Properties props = new Properties(FileTemplateManager.getInstance().getDefaultProperties(directory.getProject()));
Project project = directory.getProject();
final Properties props = new Properties(FileTemplateManager.getInstance(project).getDefaultProperties());
props.setProperty(FileTemplate.ATTRIBUTE_NAME, className);
props.setProperty(FileTemplate.ATTRIBUTE_PACKAGE_NAME, packageName);

final FileTemplate template = FileTemplateManager.getInstance().getInternalTemplate(templateName);
final FileTemplate template = FileTemplateManager.getInstance(project).getInternalTemplate(templateName);

return FileTemplateUtil.createFromTemplate(template, className, props, directory, classLoader);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected String getActionName(PsiDirectory directory, String newName, String te
@Override
protected void buildDialog(Project project, PsiDirectory directory, CreateFileFromTemplateDialog.Builder builder) {
builder.setTitle(IdeBundle.message("action.create.new.class"));
for (FileTemplate fileTemplate : HaxeFileTemplateUtil.getApplicableTemplates()) {
for (FileTemplate fileTemplate : HaxeFileTemplateUtil.getApplicableTemplates(project)) {
final String templateName = fileTemplate.getName();
final String shortName = HaxeFileTemplateUtil.getTemplateShortName(templateName);
final Icon icon = HaxeFileTemplateUtil.getTemplateIcon(templateName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected boolean isAvailable(DataContext dataContext) {
@Override
protected void buildDialog(Project project, PsiDirectory directory, CreateFileFromTemplateDialog.Builder builder) {
builder.setTitle(HaxeBundle.message("create.nmml.file.action"));
for (FileTemplate fileTemplate : HaxeFileTemplateUtil.getNMMLTemplates()) {
for (FileTemplate fileTemplate : HaxeFileTemplateUtil.getNMMLTemplates(project)) {
final String templateName = fileTemplate.getName();
final String shortName = HaxeFileTemplateUtil.getTemplateShortName(templateName);
builder.addKind(shortName, icons.HaxeIcons.Nmml_16, templateName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,41 @@
*/
package com.intellij.plugins.haxe.ide.module;

import com.intellij.execution.RunManager;
import com.intellij.execution.RunnerAndConfigurationSettings;
import com.intellij.execution.configurations.ConfigurationFactory;
import com.intellij.execution.configurations.ConfigurationTypeUtil;
import com.intellij.execution.configurations.RunConfiguration;
import com.intellij.ide.util.projectWizard.JavaModuleBuilder;
import com.intellij.ide.util.projectWizard.ModuleBuilderListener;
import com.intellij.ide.util.projectWizard.SourcePathsBuilder;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.fileEditor.FileEditorManager;
import com.intellij.openapi.module.Module;
import com.intellij.openapi.module.ModuleType;
import com.intellij.openapi.options.ConfigurationException;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.projectRoots.SdkTypeId;
import com.intellij.openapi.roots.CompilerModuleExtension;
import com.intellij.openapi.roots.ModifiableRootModel;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VfsUtil;
import com.intellij.openapi.vfs.VfsUtilCore;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.VirtualFileManager;
import com.intellij.plugins.haxe.config.HaxeTarget;
import com.intellij.plugins.haxe.config.sdk.HaxeSdkType;
import com.intellij.plugins.haxe.ide.HaxeFileTemplateUtil;
import com.intellij.plugins.haxe.module.impl.HaxeModuleSettingsBaseImpl;
import com.intellij.plugins.haxe.runner.HaxeApplicationConfiguration;
import com.intellij.plugins.haxe.runner.HaxeRunConfigurationType;
import com.intellij.psi.PsiManager;
import com.intellij.util.PathUtil;
import org.jetbrains.annotations.NotNull;

import java.io.File;
import java.util.List;

public class HaxeModuleBuilder extends JavaModuleBuilder implements SourcePathsBuilder, ModuleBuilderListener {
@Override
public void setupRootModel(ModifiableRootModel modifiableRootModel) throws ConfigurationException {
Expand All @@ -51,6 +74,75 @@ public void moduleCreated(@NotNull Module module) {
final CompilerModuleExtension model = (CompilerModuleExtension)CompilerModuleExtension.getInstance(module).getModifiableModel(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

So, the idea is that every time a module is created, we simply go ahead and create a Main.hx class in it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested it and seems like you are right

model.setCompilerOutputPath(model.getCompilerOutputUrl());
model.inheritCompilerOutputPath(false);

final Project project = module.getProject();
List<Pair<String, String>> sourcePaths = getSourcePaths();
String srcPath = null;

if ((sourcePaths.size() > 0)) {
srcPath = sourcePaths.get(0).getFirst();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We get src folder path

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not so thrilled about just adding it to the first source path. In fact, I'm not sure that we should be generating a file at all. I would think that adding a main method to an existing class would be enough -- and the more useful -- help. (And, in reality, I thought you were only going to put up a warning dialog.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is more friendly, because you would have to create Main class/method/file anyway (at least I do that, especially when I am developing this=) there are should be a way to create unit tests for it )
If a warning then should be QuickFix

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of a QuickFix to add a main() method to a class. I guess I don't really mind the ability to add a new class, either. The trouble is that we're creating that class every time we create a new module. And new modules aren't necessarily empty; they can be created from existing sources. Always adding a new file is what really bothers me. I think this functionality can exist, but NOT in the module create function.

VirtualFile dir = VirtualFileManager.getInstance().findFileByUrl(VfsUtilCore.pathToUrl(srcPath));

if (dir != null) {
try {
VirtualFile mainClassFile = dir.findFileByRelativePath("Main.hx");
if (mainClassFile == null) {
HaxeFileTemplateUtil
.createClass("Main", "", PsiManager.getInstance(project).findDirectory(dir), "HaxeMainClass",
null);
}
}
catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure you want to catch /every/ exception? I'd much prefer catching only the exception that the code can throw. Also, if we just continue after the exception, then we are changing settings to work with a non-existent class. The best course of action is to catch recoverable errors and let the rest fly past.

e.printStackTrace();
}
}

HaxeModuleSettings moduleSettings = HaxeModuleSettings.getInstance(module);
moduleSettings.setMainClass("Main");
moduleSettings.setArguments("");
moduleSettings.setNmeFlags("");
moduleSettings.setHaxeTarget(HaxeTarget.NEKO);
Copy link
Contributor

Choose a reason for hiding this comment

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

I really dislike forcing the project into a different mode just because we added a class. That doesn't make any sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is mostly default settings, not much is changed - mainly Main class stuff

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't you rather duplicate the settings from related modules. I mean, if the user is targeting OpenFL, shouldn't we make that the default for the new module as well?

But maybe we never did that in the first place...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is interesting, I just don't use multiple modules, so I don't know how it is in practical sense

Copy link
Contributor

Choose a reason for hiding this comment

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

Out project(s) have something like 80 modules, depending upon the target. Basically, we think of modules as libraries. We have them compile into their own haxelibs, but they are part of the larger project so that we can traverse the sources easily. Each one of them should not have a main class.

From: as3boyan [mailto:[email protected]]
Sent: Tuesday, February 02, 2016 10:46 AM
To: TiVo/intellij-haxe
Cc: Eric Bishton
Subject: Re: [intellij-haxe] CreateMainClassAndOpenItInEditor (#398)

In src/common/com/intellij/plugins/haxe/ide/module/HaxeModuleBuilder.javahttps://github.com//pull/398#discussion_r51614372:

  •      if (mainClassFile == null) {
    
  •        HaxeFileTemplateUtil
    
  •          .createClass("Main", "", PsiManager.getInstance(project).findDirectory(dir), "HaxeMainClass",
    
  •                       null);
    
  •      }
    
  •    }
    
  •    catch (Exception e) {
    
  •      e.printStackTrace();
    
  •    }
    
  •  }
    
  •  HaxeModuleSettings moduleSettings = HaxeModuleSettings.getInstance(module);
    
  •  moduleSettings.setMainClass("Main");
    
  •  moduleSettings.setArguments("");
    
  •  moduleSettings.setNmeFlags("");
    
  •  moduleSettings.setHaxeTarget(HaxeTarget.NEKO);
    

That is interesting, I just don't use multiple modules, so I don't know how it is in practical sense


Reply to this email directly or view it on GitHubhttps://github.com//pull/398/files#r51614372.


This email and any attachments may contain confidential and privileged material for the sole use of the intended recipient. Any review, copying, or distribution of this email (or any attachments) by others is prohibited. If you are not the intended recipient, please contact the sender immediately and permanently delete this email and any attachments. No employee or agent of TiVo Inc. is authorized to conclude any binding agreement on behalf of TiVo Inc. by email. Binding agreements with TiVo Inc. may only be made by a signed written agreement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, yeah, it needs to be flexible

moduleSettings.setExcludeFromCompilation(false);
moduleSettings.setBuildConfig(HaxeModuleSettingsBaseImpl.USE_PROPERTIES);
moduleSettings.setOutputFileName("Main.n");
String outputFolder = PathUtil.toSystemIndependentName(project.getBasePath() + "/out/production");
moduleSettings.setOutputFolder(outputFolder);
//String releaseFolder = PathUtil.toSystemIndependentName(outputFolder + "/release");
//boolean mkdirs = new File(releaseFolder).mkdirs();

String url = VfsUtil.pathToUrl(outputFolder);
model.setCompilerOutputPath(url);

assert dir != null;
final VirtualFile file = dir.findFileByRelativePath("Main.hx");
assert (file != null);

ApplicationManager.getApplication().runWriteAction(new Runnable() {
@Override
public void run() {
FileEditorManager.getInstance(project).openFile(file, true);
}
});

ConfigurationFactory[] factories =
ConfigurationTypeUtil.findConfigurationType(HaxeRunConfigurationType.class).getConfigurationFactories();

if ((factories.length > 0)) {
RunConfiguration configuration = factories[0].createTemplateConfiguration(project);
RunManager manager = RunManager.getInstance(project);
RunnerAndConfigurationSettings runnerAndConfigurationSettings = manager.createConfiguration(configuration, factories[0]);
RunConfiguration configuration1 = runnerAndConfigurationSettings.getConfiguration();
if (configuration1 instanceof HaxeApplicationConfiguration) {
HaxeApplicationConfiguration haxeApplicationConfiguration = (HaxeApplicationConfiguration)configuration1;
haxeApplicationConfiguration.setModule(module);
}
manager.addConfiguration(runnerAndConfigurationSettings, false);
manager.setSelectedConfiguration(runnerAndConfigurationSettings);
}


}

model.commit();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,26 @@
*/
package com.intellij.plugins.haxe.ide.module;

import com.intellij.ide.fileTemplates.FileTemplateManager;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that we need to add this file with only import changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I did a clean up

import com.intellij.ide.fileTemplates.FileTemplateUtil;
import com.intellij.ide.util.projectWizard.ModuleWizardStep;
import com.intellij.ide.util.projectWizard.ProjectJdkForModuleStep;
import com.intellij.ide.util.projectWizard.WizardContext;
import com.intellij.openapi.fileEditor.impl.PsiAwareFileEditorManagerImpl;
import com.intellij.openapi.module.ModuleType;
import com.intellij.openapi.module.ModuleTypeManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.roots.ui.configuration.ModulesProvider;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.VirtualFileManager;
import com.intellij.plugins.haxe.HaxeBundle;
import com.intellij.plugins.haxe.HaxeFileType;
import com.intellij.plugins.haxe.config.sdk.HaxeSdkType;
import com.intellij.plugins.haxe.hxml.HXMLFileTypeFactory;
import com.intellij.plugins.haxe.ide.HaxeFileTemplateUtil;
import com.intellij.psi.PsiFileFactory;
import com.intellij.psi.PsiManager;
import com.intellij.psi.impl.file.PsiDirectoryFactory;

import javax.swing.*;

Expand Down