|
From: Raphael N. <rne...@hs...> - 2012-07-20 17:53:08
|
On Wed, Jul 18, 2012 at 10:15:13PM +0200, Hannes Schüller wrote:
> Hello Raphael!
>
> Raphael Nestler writes:
> > I this patch series adds some changes to the browsersettings code. I
> > think using a single enum instead of three different booleans is much
> > more intuitive.
>
> It probably is, yes.
>
> > To be able to use the give_feedback() function I needed to set focus
> > to the webview widget, otherwise it wouldn't show anything. I consider
> > this a bug, since it also doesn't show anything when entering an
> > invalid setting.
>
> Hm, well, let's say it is an unwanted side-effect. give_feedback()
> simply points to echo(). The check there whether the input field is
> focused makes perfect sense in my opinion, because otherwise, it might
> just overwrite what the user is typing. After confirming a command line,
> I agree that feedback should be activated, though.
So we need to remove focus from the input field as soon as the command
line gets confirmed. Shouldn't this happen automatically by Gtk anyway?
>
> > The third patch saves the value for boolean settings into the .var field of
> > the setting. I need to do this as well for integer settings, but I wanted to
> > receive some feedback on the patch in general first. I also thought about
> > using
> > something like:
> > struct {
> > ESetingsType type;
> > union value {
> > char *s;
> > int i;
> > gboolean b;
> > };
> > }
> > for the settings to not be forced to save every value as a string.
>
> I don't know if you're aware, but for the "per site settings" patch, I
> made all settings into strings internally intentionally. I don't like
> this much, but this seemed to be the easiest way to avoid the code to be
> specific for each individual case. Would be interesting to consolidate
> with your approach.
I took a look at the per site settings patch, but it didn't apply to
master cleanly:
Applying: moving all internal flags into a new global struct of the type VSettings
vimprobable/.git/rebase-apply/patch:415: space before tab in indent.
strncat(temp, file_url, 1);
vimprobable/.git/rebase-apply/patch:416: space before tab in indent.
file_url++;
error: patch failed: config.h:1
error: config.h: patch does not apply
Patch failed at 0001 moving all internal flags into a new global struct of the type VSettings
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Am I doing something wrong? Or should the patches be rebased to current master first?
Raphael
|
