awhawks / workspacemechanic

Automatically exported from code.google.com/p/workspacemechanic
0 stars 0 forks source link

Allow ~ to be used to refer to tasks relative to the user's home directory #61

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
When specifying multiple source repositories as a JSON array, it is not 
possible to have a directory relative to the user's home directory.

sourceDirectories=["http\://some.site.com","~/.eclipse/mechanic"]

It is also not possible to use the variable substitution, because that doesn't 
kick in for JSON arrays.

Instead, change FileTaskProvider so that if it is supplied with a relative 
path, it will replace that with the contents of ${user.dir} and for home 
directories, replace ~ with ${user.home}.

Original issue reported on code.google.com by alex.ble...@gmail.com on 20 Jul 2011 at 4:31

Attachments:

GoogleCodeExporter commented 8 years ago
This patch looks good, but I suggest each  path.startsWith have a 
path.separator follow it.

e.g.

private static final String SEPARATOR=System.getProperty("path.separator");

    if(path.startsWith("~" + SEPARATOR)) {
      dir = new File(System.getProperty("user.home"), path.substring(1));
    } else if (path.startsWith(".." + SEPARATOR)) {
      dir = new File(new File(System.getProperty("user.dir"),".."), path.substring(2));
    } else if (path.startsWith("." + SEPARATOR)) {

WDYT?

Original comment by konigsb...@gmail.com on 10 Sep 2011 at 4:03

GoogleCodeExporter commented 8 years ago
Yes, that's definitely required. ..foo doesn't make sense, but ~foo does. 
Perhaps support for ~foo as well? But not now.

Original comment by konigsb...@gmail.com on 10 Sep 2011 at 4:05

GoogleCodeExporter commented 8 years ago
~ is not working on my mac.

Expected
/Users/robertkonigsberg/foo
/Users/robertkonigsberg/Documents/workspace/workspacemechanic/plugins/com.google
.eclipse.mechanic.tests/~/foo

Interesting... Maybe I hacked it wrong.

Original comment by konigsb...@gmail.com on 10 Sep 2011 at 4:07

GoogleCodeExporter commented 8 years ago
Ah, yes, file.separator, not path.separator.

http://download.oracle.com/javase/tutorial/essential/environment/sysprop.html

Original comment by konigsb...@gmail.com on 10 Sep 2011 at 4:10

GoogleCodeExporter commented 8 years ago

Original comment by konigsb...@gmail.com on 10 Sep 2011 at 4:18

GoogleCodeExporter commented 8 years ago
I don't think you should require a / there. For a start, using the paths in 
unix stye works on windows, but not vice versa. currently I have 
~/.eclipse/mechanic which works on both Windows and Mac - if you introduce the 
separator as suggested you are preventing a cross-platform string from being 
used (which was the whole point of introducing this change).

If you are going to do that, I suggest instead of string concat on each test 
you do a .charAt(2) == '/' or '\\' which permits both systems to use the same 
syntax and avoid unnecessary concatenations. 

Original comment by alex.ble...@gmail.com on 10 Sep 2011 at 8:46

GoogleCodeExporter commented 8 years ago
Can you reopen? Or shall I file a new bug?

Original comment by alex.ble...@gmail.com on 10 Sep 2011 at 8:58

GoogleCodeExporter commented 8 years ago
I've committed the altered change, feel free to synchronize.

I agree with your assessment.

Original comment by konigsb...@gmail.com on 10 Sep 2011 at 11:11

GoogleCodeExporter commented 8 years ago
I checked out the latest version and it seems the test works for both Windows 
and OSX. This seems to be because the File constructor converts whatever the 
input path is to the appropriate file-system specific path; so checking for 
file.separator is entirely correct in this case, regardless of what the input 
type happens to be (providing the tit can convert). It means that Windows can 
handle c:\ or c:/, but presumably Linux will handle / better than \.

So I'm ok with this change as is.

Original comment by alex.ble...@gmail.com on 3 Oct 2011 at 7:03

GoogleCodeExporter commented 8 years ago

Original comment by konigsb...@gmail.com on 3 Oct 2011 at 7:12