- Dillo-dev - lists.mailman3.com

[patch] Configurable key bindings

by tim.nieradzik＠gmx.de

> - This change is not backwards compatible. Do not read old dillorc > format and it is a big inconvenience for users. No, the new format is not backward compatible but it supports the equal signs which were used in the old format for associations. Dillo is mostly used by experienced Linux users who should not have any problems adding a simple "set " before each line. Backward compatibility should never be used as an argument against enhancements. Otherwise programs would just become larger and larger. They carry various compatibility layers without any real benefits. The more compatibility code an application contains, the more difficult it is to maintain it. > - Can delete some meta info stored in dillorc. Do you mean that the migration ``tool'' may delete comments? The dillorc is not migrated automatically (even though writing a small scrip would not be too difficult). As the user has to do this manually, it is one's choice whether to keep the comments or not. > - There are compatible ways to add this preferences (key_save = "...", > key_find = "CTRL + f", ...) Sure, but options were not initially thought of as being used for key bindings. When using a special keyword which is only valid for one preferences type (bindings), it is possible to add further parameters such as groups (useful when having lots of key bindings). The other advantage of this implementation is that we could easily modify it to allow multiple key bindings for one symbol. > - If the number of key bindings increase much another config file can > be convenient. With another config file some example files can be > suplied (vim keys, firefox keys, ...) and end user only need to copy > the prefered file. The idea of having several ``preset'' files for key bindings is well. In my opinion, we should give the user the right to decide whether to put everything into one file or to separate the configuration into multiple files. Therefore an "include" command for the dillorc should be implemented. Then we could put something like "include /usr/share/dillo/bindings/default" there, as well as some others. Now, if somebody wants to try different key bindings, you would just have to uncomment this particular line. There is no need for replacing your local bindings just to try others out. --Tim

Feb. 24, 2009

[patch] Configurable key bindings

by tim.nieradzik＠gmx.de

> I think I'd feel better if it could be used like prefs, > that is, isPressed testing bindings.reload instead of > looking for the binding associated with the "reload" string. Personally, I also like this concept better as it reduces the risk of misspellings (the compiler would throw an error). Furthermore, the code looks much cleaner and it is likely that it become a bit faster (because seeking the "symbols" array is not be necessary anymore). But on the other hand there needs to be a large mapping table that contains all symbol names twice for each entry (like it is done in src/prefs.c). This was the main reason why I decided against a "bindings" array because I simply wanted to avoid duplicate code. To find balance between these two approaches, I would suggest altering isPressed() requiring only the position of the symbol in the "symbols" array. Then we could create a constant for each binding containing that ID. It works similar to prefs' concept except that we only map integers. Hence, when adding new entries, we have to repeat the symbol name less frequent. The only problem with this approach is that it easily breaks when the items are accidentally re-ordered. --Tim

Feb. 24, 2009

patches, bundles, etc

by dillo-dev＠toykeeper.net

BTW, what's the policy on submitting sizable changes with multiple revisions, such as the configurable keybindings? Is it okay to send a changeset bundle, or do the changes need to be flattened into a single patch? It probably doesn't matter for small changes, but it might be nice to include the history for large changes. -- Scott

Feb. 24, 2009

[patch] Configurable key bindings

by dillo-dev＠toykeeper.net

* Tim Nieradzik <tim.nieradzik(a)gmx.de> wrote: > It took longer to write this patch than I expected. That really was quick! I'm excited to see this. I'm not very familiar with Dillo's code, so I only have superficial comments. Mostly, I see some keys missing... > +struct mapping_s keyNames[] = { > + { "Backspace", fltk::BackSpaceKey }, > + { "Delete", fltk::DeleteKey }, > + { "Down", fltk::DownKey }, > + { "End", fltk::EndKey }, > + { "Esc", fltk::EscapeKey }, > + { "F1", fltk::F1Key }, > + { "F2", fltk::F2Key }, > + { "F3", fltk::F3Key }, > + { "F4", fltk::F4Key }, > + { "F5", fltk::F5Key }, > + { "F6", fltk::F6Key }, > + { "F7", fltk::F7Key }, > + { "F8", fltk::F8Key }, > + { "F9", fltk::F9Key }, F10 through F20? > + { "Home", fltk::HomeKey }, > + { "Insert", fltk::InsertKey }, > + { "Left", fltk::LeftKey }, > + { "PageDown", fltk::PageDownKey }, > + { "PageUp", fltk::PageUpKey }, > + { "Print", fltk::PrintKey }, > + { "Return", fltk::ReturnKey }, > + { "Right", fltk::RightKey }, > + { "Space", fltk::SpaceKey }, > + { "Tab", fltk::TabKey }, > + { "Up", fltk::UpKey } > +}; Keypad keys? > +static struct mapping_s modifierNames[] = { > + { "Shift", fltk::SHIFT }, > + { "Ctrl", fltk::CTRL }, > + { "Alt", fltk::ALT }, > + { "Button1", fltk::BUTTON1 }, > + { "Button2", fltk::BUTTON2 }, > + { "Button3", fltk::BUTTON3 } > +}; What about Meta, Hyper, Super, AltGr, and Modeshift? Or, more generally, Mod1 through Mod5? BTW, I know it was like this before too, but something to think about for later is that this section could probably be replaced by a table instead of a bunch of if/else statements. > + if ((Keys::isPressed("hide-panels")) && (get_panelmode() == UI_TEMPORARILY_SHOW_PANELS)) { > + set_panelmode(UI_HIDDEN); > + ret = 1; > + } else if (Keys::isPressed("bookmarks")) { > + a_UIcmd_book(a_UIcmd_get_bw_by_widget(this)); > + ret = 1; > + } else if (Keys::isPressed("find")) { > + set_findbar_visibility(1); > + ret = 1; > + } else if (Keys::isPressed("goto")) { > ... > + } else if (Keys::isPressed("back")) { > + a_UIcmd_back(a_UIcmd_get_bw_by_widget(this)); > + ret = 1; > + } else if (Keys::isPressed("forward")) { > + a_UIcmd_forw(a_UIcmd_get_bw_by_widget(this)); > + ret = 1; > } And, if you want to have an easier time keeping the formatting consistent, try this in your vimrc: " show tabs and trailing spaces set listchars=tab:>-,trail:- set list It's a minor thing really, but I noticed some inconsistent whitespace, and those config lines make it really easy to spot. :) -- Scott

Feb. 24, 2009

[patch] Configurable key bindings

by corvid＠lavabit.com

Diego wrote: > - If the number of key bindings increase much another config file can > be convenient. With another config file some example files can be > suplied (vim keys, firefox keys, ...) and end user only need to copy > the prefered file. That's a good point.

Feb. 24, 2009

[patch] Configurable key bindings

by darkspirit5000＠gmail.com

I disagree about changing the dillorc format: - This change is not backwards compatible. Do not read old dillorc format and it is a big inconvenience for users. - Can delete some meta info stored in dillorc. - There are compatible ways to add this preferences (key_save = "...", key_find = "CTRL + f", ...) - If the number of key bindings increase much another config file can be convenient. With another config file some example files can be suplied (vim keys, firefox keys, ...) and end user only need to copy the prefered file. I do not known what other option is best, but i think that this format change is not needed. Diego. 2009/2/24, corvid <corvid(a)lavabit.com>: > I think I'd feel better if it could be used like prefs, > that is, isPressed testing bindings.reload instead of > looking for the binding associated with the "reload" string. > > > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev >

Feb. 24, 2009

[patch] Configurable key bindings

by corvid＠lavabit.com

I think I'd feel better if it could be used like prefs, that is, isPressed testing bindings.reload instead of looking for the binding associated with the "reload" string.

Feb. 24, 2009

[patch] Configurable key bindings

by corvid＠lavabit.com

Tim wrote: > > looking in events.h, I think > > (event_state() & (SHIFT | CTRL)) would work. > > Good idea. It does not work as expected though. But this does: > ((event_state() & (SHIFT | CTRL)) == (SHIFT + CTRL)) err, yes :) or probably event_state() == (SHIFT | CTRL) in this case. > Now that we know how to check for multiple modifiers, does somebody > want to implement it? I lean toward not adding features to this patch yet. (Continuing to add features before getting anything into the tree caused dillo not to get frames support, and that was an unfortunate incident all around.)

Feb. 23, 2009

[patch] Configurable key bindings

by tim.nieradzik＠gmx.de

> looking in events.h, I think > (event_state() & (SHIFT | CTRL)) would work. Good idea. It does not work as expected though. But this does: ((event_state() & (SHIFT | CTRL)) == (SHIFT + CTRL)) Now that we know how to check for multiple modifiers, does somebody want to implement it? --Tim

Feb. 23, 2009

[patch] Configurable key bindings

by corvid＠lavabit.com

Tim wrote: > > - I'm sure that Jorge isn't going to want exception handling. > > Why not? Exceptions are more flexible when it comes to longer and > more complicated functions. Of course, we could also return an > integer status code but sometimes it is necessary to return > non-integer values. My Jorge-simulator might be imperfect, but we'll see. (When I did most of my C++, the compiler didn't even have exceptions yet, so I can't claim bitter experience, but they've always looked a bit like gotos to me when it comes to maintainability.) > > PS Are you sure that fltk doesn't allow modifiers to be combined? > > Grepping through fltk/test, it looks like they're combining them > > in menubar.cxx. > > It looks like this is possible indeed. However, I used > event_state(SHIFT & CTRL) to check whether it recognizes a combined > Shift and Ctrl but this did not work for me. Any ideas? looking in events.h, I think (event_state() & (SHIFT | CTRL)) would work.

Feb. 23, 2009