-
Notifications
You must be signed in to change notification settings - Fork 88
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
Unable to launch Eclipse launch config if name contains spaces #1630
Comments
I'm looking into it. Reverting a series of commits is not what I aim at . |
It is one commit + one change from other commit. Should be safe. |
This reverts commit e62be0b because it caused regression, see eclipse-pde#1630
Partial revert of 04785c9 Fixes eclipse-pde#1630
See #1631. |
@iloveeclipse I am unable to reproduce the problem on Linux . Do you see it on windows? |
Linux |
Note, PDE creates two directories with different names but deletes only one. To reproduce, one has manually delete all existing directories. |
I'm not sure this is the place to fix, because launcher cant know whether I believe the problem is in PDE where the paths entered in the UI and paths passed to the launcher differ. |
It's really quite the disaster area. The launcher main is so terrible with all the confusion and hacks around things that are URLs or maybe they are file paths, or maybe goodness knows maybe both, neither, some abomination in between that is not even a valid URI this is kind of a semi-bogus URL. I found that this fixes the problem as far as I've been able to reproduce it. |
- First try to create a URI from the spec and then create a File from that URI to ensure that the spec is properly decoded. eclipse-pde/eclipse.pde#1630
I can confirm the PR with the fix above fixes this issue. |
- First try to create a URI from the spec and then create a File from that URI to ensure that the spec is properly decoded. eclipse-pde/eclipse.pde#1630
- First try to create a URI from the spec and then create a File from that URI to ensure that the spec is properly decoded. eclipse-pde/eclipse.pde#1630
Thanks Ed. |
By the way is there any chance to add a regression test so we notice such thing earlier and don't need manual testing? |
What's the standard answer to that type of question? 😬 In any case, that code, I'm sorry to say, is kind of a disaster area. Unless all places create properly encoded URIs in the first places and all places converting those to file system paths decode those properly I think it will never work 100% correctly, e.g., don't try this at home boys and girls: When I see a lot of So not only do there need to be tests, there needs to be significant rework to stamp out the sources of bad data... |
I think a test that uses a space in the path would be a very first step. |
Regression in 4.35 via #1565.
New_configuration
)New_configuration (1)
(note the space)Log file contains this:
Reverting e62be0b fixes the launch.
The text was updated successfully, but these errors were encountered: