You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/04/22 05:34:22 UTC
[GitHub] [netbeans] errael opened a new pull request #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
errael opened a new pull request #2097:
URL: https://github.com/apache/netbeans/pull/2097
…PARENT_FOLDER
Problem is the settings property key "projdir", aka PROJECT_PARENT_FOLDER, used by maven's new project wizard's "Name and Location" panel. "projdir" has two meaning.
- Maven's CreateNewModule action uses "projdir" property to inform the wizard where to put the project
example: /tmp/parent
- The wizard uses "projdir" property to inform project creation of the project directory
example: /tmp/parent/child
This difference is a problem if back/next are used in the dialog; in this case the output `/tmp/parent/child` becomes the input. Repeated back/next cause the directory to grow each time. See #1980 for details of the first fix and discussion of problem with CreateNewModule action. The first fix took care of the back/next problem, but it did not take CreateNewModule usage into account.
This fix detects the first time the properties are read, and in this case uses "projdir" to indicate the parent directory; subsequent usage (back/next) "projdir" is used to indicate the child directory.
This is a quick fix for 12.0. NOTE that most old code uses literal "projdir", rather than PROJECT_PARENT_FOLDER. I suggest a new property be introduced, for example
```
String PROJECT_BASE_FOLDER = "projdir";
String PROJECT_PARENT_FOLDER = "parent-projdir";
```
@neilcsmith-net , @matthiasblaesing this needs some support to get it into 12.0
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
errael commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-617901364
> Also, are the `PROJECT_PARENT_FOLDER` and the target folder meant to be the same? _I think_ both are meant to be the parent folder of the created project?
Do you still think this is the case?
> I'm not entirely sure it hasn't broken other things now I've started looking further.
Could you clarify what you've seen from "looking further"?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on pull request #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-618943737
> Yeah, that fix was to avoid directory creation when Cancel.
That was my point - it was only creating a directory at all because it was passing the wrong argument.
Still, thanks for fixing my issue with this for the release. Post 12.0, and if Maven support is moved away from being friend-only, then it should probably be revised to be in spec.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
errael commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-617904852
The current contract of the maven wizard panel is
- at initialization use "projdir" as the parent
- at finish set "projdir" as the target projectDirectory
The previous change and this PR respect that contract, AFAICT.
I believe the contract is flawed (and I think others agree). I've suggested a concept which I think impove it. See the end of the original statement https://github.com/apache/netbeans/pull/2097#issue-407057487 . There may be much better solutions.
In any event, something must be done to fix the serious bug that was found,
either revert the original change or merge this change. (or ...)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
errael commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-617793629
> Can you document where and how the "projdir" attribute in the wizard is used elsewhere that requires it to be the project folder and not the parent?
I can probably find it again. Here's an easy way to see it. In `store()` set "projdir"/PROJECT_PARENT_FOLDER property to the same value as picked up in `read()`, "/tmp/parent" in the example. Then after "Finish" and project creation, there is a "/tmp/parent/src" and the parent project is marked with "unloadable" (or somesuch) (that is the first thing I tried, as @neilcsmith-net suggested and I agreed)
Note that before the fix for the next/prev bug, "projdir" was set to "/tmp/parent/child" in `store()`, reverting the fix will not change that. before any changes. This is from
```
changeset: 1692:b319f80ce20b
summary: Moving java modules into java subdirectory
```
Here's the code in `BasicPanelVisual.store()`
```
String name = projectNameTextField.getText().trim();
String folder = createdFolderTextField.getText().trim();
final File parentFolder = new File(folder);
d.putProperty(CommonProjectActions.PROJECT_PARENT_FOLDER, parentFolder);
```
Even though the variable says `parentFolder` it comes from `createdFolderTextField`. I changed the names around to satisfy a request from reviewer in https://github.com/apache/netbeans/pull/1980#discussion_r387210839
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-617954226
> Do you still think this is the case?
Yes, because target folder on the template takes a `DataFolder`, which means the thing you're trying to create must already exist, which seems backwards?! I have a feeling the `mkdirs()` call that deep is also the cause of another archetype bug report?
Still, you're right about the existing wizard panel contract, so I guess maybe this is the right fix for 12.0 - I think it will fix the issues I've seen - did more digging and not seen another problem, except more suspicious uses of that parameter.
I'm not sure I agree that the contract is flawed though - Gradle support seems to be doing what the Maven wizards should be doing? https://github.com/apache/netbeans/blob/master/groovy/gradle/src/org/netbeans/modules/gradle/spi/newproject/SimpleGradleWizardIterator.java#L85
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
errael commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-618075046
> Yes, because target folder on the template takes a DataFolder, which means the thing you're trying to create must already exist, which seems backwards?! I have a feeling the mkdirs() call that deep is also the cause of another archetype bug report?
If you're talking about the `((TemplateWizard) d).setTargetFolderLazy(`, this is recent, and part of an API change introducing the Lazy version. Before this, if Cancel button used, directories were left scattered around; particularly ugly if back/next had been used leaving a chain of empty directories. In BasicPanelVisual
```
d.putProperty(CommonProjectActions.PROJECT_PARENT_FOLDER, parentFolder);
if (d instanceof TemplateWizard) {
- parentFolder.mkdirs();
- ((TemplateWizard) d).setTargetFolder(DataFolder.findFolder(FileUtil.toFileObject(parentFolder)));
+ ((TemplateWizard) d).setTargetFolderLazy(() -> {
+ parentFolder.mkdirs();
+ return DataFolder.findFolder(FileUtil.toFileObject(parentFolder));
+ });
}
````
If you're curious about the full change
```
changeset: 3987:b9e1311c588c
user: Ernie Rael <er...@raelity.com>
date: Sat Feb 29 21:41:39 2020 +0000
summary: [NETBEANS-3918] defer creation of target folder until/if needed
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
errael commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-617899633
Two extreme possibilities are that "as documented" is correct and all
the code is wrong. Or vice-versa. We're probably somewhere in the middle,
but closer to "as documented" wrong I think.
When working on the original fix, I saw how the GUI and
instantiation were copied/modeled from java projects. In
```
java/java.j2semodule/src/.../NewJ2SEModularProjectWizardIterator.java
```
note the explicit use of "projdir" in the following
```
public Set<FileObject> instantiate (ProgressHandle handle) throws IOException {
...
File dirF = (File)myWiz.getProperty("projdir"); //NOI18N
...
J2SEModularProjectGenerator.createProject(dirF, name, platform);
```
And in this code from
```
java/maven/.../newproject/idenative.IDENativeMavenWizardIterator
```
note the usage of "projdir" (PROJECT_PARENT_FOLDER)
```
public Set<FileObject> instantiate (ProgressHandle handle) throws IOException {
...
File projFile = FileUtil.normalizeFile((File) wiz.getProperty(
CommonProjectActions.PROJECT_PARENT_FOLDER)); // NOI18N
...
CreateProjectBuilder builder = createBuilder(projFile, vi, handle);
```
I put a breakpoint on CreateProjectBuilder and
- Did CreateNewModule on pom project /tmp/parent,
- projectName called child,
- dialog shows projectLocation as /tmp/parent
- and projectFolder as /tmp/parent/child
at the breakpoint: projFile is /tmp/parent/child
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-617840353
> I can probably find it again. Here's an easy way to see it. In store() set "projdir"/PROJECT_PARENT_FOLDER property to the same value as picked up in read(), "/tmp/parent" in the example. Then after "Finish" and project creation, there is a "/tmp/parent/src" and the parent project is marked with "unloadable"
Yes, not entirely surprising - something is relying on the incorrect (as documented) value. That is where the fix should be IMO. Also, are the `PROJECT_PARENT_FOLDER` and the target folder meant to be the same? I _think_ both are meant to be the parent folder of the created project?
Maybe can be fixed up as part of the wizard instantiate?
Personally, given that the prev/next bug fix introduced something worse, I'd still rather revert and review post 12.0 than risk adding to it and leaving a problem in there. But see what others think.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-618204801
> If you're talking about the ((TemplateWizard) d).setTargetFolderLazy(, this is recent, and part of an API change introducing the Lazy version.
No, I'm not. I'm aware of that change. But after looking more at this issue, my reading of TemplateWizard is that this is intended to be the folder _in which_ the new file or project is created, the parent of the project itself. I'm not sure the lazy API actually was needed to fix the prev/next folder creation issue, although useful if typing in parent name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on issue #2097: [NETBEANS-4206] Only set project location once directly from PROJECT_…
Posted by GitBox <gi...@apache.org>.
errael commented on issue #2097:
URL: https://github.com/apache/netbeans/pull/2097#issuecomment-618495664
> not sure the lazy API actually was needed to fix the prev/next folder creation issue
Yeah, that fix was to avoid directory creation when Cancel. prev/next just made it a spectacular failure.
I've never worked directly with TemplateWizard, hope it stays that way ;-) I'd guess newer stuff, spi.project.ui..., got glued to modified ant java stuff, but wasn't lined up quite right.
The new stuff using "projdir" seems like a mistake; or at least a poor choice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists