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 |