From: Sebastian M. <seb...@un...> - 2007-10-13 15:55:00
Attachments:
addopts.patch
|
Hi I just added another field to the preferences called "additional options". It allows to supply a string that is appended to the gvim start command. I want to use it for "-u ~/.vprc" which contains the following ATM: --- " include normal .vimrc source .vimrc " remove ugly toolbar and menu set guioptions-=T set guioptions-=m " more eclipse-specific settings to come " ... --- I had to rework the server-start method a bit, so please review the patch I attach below before I commit it. Sebastian. |
From: David T. <dav...@gm...> - 2007-10-17 15:13:48
|
Hi Sebastian, [Sebastian Wrote:] > I just added another field to the preferences called "additional > options". It allows to supply a string that is appended to the gvim > start command. I want to use it for "-u ~/.vprc" which contains the > following ATM: Sure sounds like a nice idea. > I had to rework the server-start method a bit, so please review the > patch I attach below before I commit it. Ok, well I haven't really dealt with patches much in the past, so I'll just describe what changes I think should be made. 1. The default value should be blank. 2. I think the title should be: "Additional Vim Parameters" as opposed to "additional Parameter" 3. Your changes need to be also made in the other start method: "public void start(AbstractVimEditor editor)" 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). 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'" Also, I think we should probably provide a tooltip for this option since it's hard to come up with a clear short name that leaves no doubt in the users mind as to what it's for. The problem is that I couldn't find any way to use tooltips with FieldEditor subclasses. The only way I could find to use tooltips involved having to use a PreferencePage as opposed to a FieldEditorPreferencePage and using standard SWT widgets such as a Button. I tried mixing a Button widget in with the FieldEditor widgets, but it caused some layout problems. For a code example of a preference page with tooltips see: org.eclipse.ui.internal.dialogs.WorkbenchPreferencePage Best Regards, David Terei |
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. |
From: David T. <dav...@gm...> - 2007-10-19 10:44:06
|
> Thanks for the valuable feedback! No trouble :) >> 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. Okay sure, we may just need to not used ProcessBuilder then, launch vim in a more manual way passing paramaters ourselves. Otherwise to distinguish spaces we will need to look for quotation marks in the string and split on them. >> 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 :-) Sure. Now that its in the repo Ill check it out myself this weekend as well. ~ David |