|
From: Sebastian M. <s....@gm...> - 2007-10-17 17:22:07
|
Thanks for the valuable feedback!
Am Thu, 18 Oct 2007 01:14:32 +1000 schrieb David Terei:
> 1. The default value should be blank.
OK, you're right.
> 3. Your changes need to be also made in the other start method: "public
> void start(AbstractVimEditor editor)"
OK.
> 4. We need the selection code for --socketid since for windows it is
> --windowid. (This is probably fixed already since I know you made a
> change for this).
yep it's fixed.
> 5. I think all the code for splitting the opt field up is not necessary.
> We can keep it as a string and append it onto our args. Also the way it
> currently splits it just on spaces is wrong since someone may have this:
> "-u 'C:\Document and Settings\davidt\_vimrc'"
^^^ ^^^
a b
OK, I fiddled around with this one a bit. The problem was that the
ProcessBuilder passed every arg as _one_ arg to the command. thus an arg
"-u
~/.vprc" didn't work, beacuse of the space. That's why I splitted
everything
up. And you are right, a file that contains spaces wont work that way.
But how can we distinguish space (a) from space (b) ??
I tell you what: Since you said it's a generally good idea, I'll commit
it, and
we fix it in the repo.
> Also, I think we should probably provide a tooltip for this option since
> ...
> For a code example of a preference page with tooltips see:
> org.eclipse.ui.internal.dialogs.WorkbenchPreferencePage
I'll have a look at it, but to be honest I'm not the GUI-kind of
developer :-)
Sebastian.
|