PoC PR to allow remember keyboard layout for each browser tab (in my case it is Firefox) #6
Conversation
| @@ -0,0 +1,70 @@ | |||
| /* | |||
| * File: lrucache.hpp | |||
| * Author: Alexander Ponomarev | |||
There was a problem hiding this comment.
It's definitely not you =)
Anyway, lets get rid of c++.
I'm sure we can extend layouts.h to support strings instead of adding a second language for such a small utility.
There was a problem hiding this comment.
True, it's not mine. It was copied from https://github.com/lamerman/cpp-lru-cache with minor changes. The use of exceptions has been removed. It has a 3-clause BSD license, which is compatible with the current project's MIT license.
It's true that layouts.h can be extended for this use case, but I really don't like the idea. It has O(n) complexity for insertion and search. And writing an efficient algorithm in C is very painful. I searched GitHub for several LRU implementations in C and they were all terrible, too complex to judge their quality and some of them can't even pass basic tests. In C++, this algorithm can be written efficiently in a few lines of code with O(1) complexity for both operations. My perfectionism does not like O(n) complexity even in simple programs)
There was a problem hiding this comment.
It has O(n) complexity for insertion and search
Even if you have 100 windows and 1000 open tabs - O(n) is not a problem.
I don't think anyone would create thousands of windows or open millions of tabs.
And writing an efficient algorithm in C is very painful
Just an example, hashmap in ~300 lines of C code: http://pokristensson.com/strmap.html
My perfectionism does not like O(n) complexity even in simple programs
My perfectionism doesn't like mixing C and C++ in a project with 5 source files :)
There was a problem hiding this comment.
I understand you) This is just for fun, we are not paid for side projects. I had fun mixing two languages together, in some production system I'll think twice about it) But anyway I just wanted to share the idea. This PR does not have to be merged into your project.
There was a problem hiding this comment.
No problem =)
Thank you anyway.
There was a problem hiding this comment.
Thanks again for this project! This really saves me a lot of time in Sway)
src/sway.c
Outdated
| * @return container Id or -1 if not found | ||
| */ | ||
| static int container_id(struct json_object* msg) | ||
| static void fill_window_name(char *w_str, size_t len, struct json_object* msg) |
There was a problem hiding this comment.
We are not actually "fill the window name" here. And not only name.
It is still a method to create unique ID. I would name it "generate_id()".
Also, please, move the msg argument to the first place. Input parameters, then outputs.
src/sway.h
Outdated
| @@ -8,13 +8,13 @@ | |||
| * @param[in] window identifier of currently focused window (container) | |||
| * @return keyboard layout to set, -1 to leave current | |||
| */ | |||
| typedef int (*on_focus)(int window); | |||
| typedef int (*on_focus)(char *window); | |||
There was a problem hiding this comment.
These functions must not modify the window buffer, so it could be const.
src/main.c
Outdated
| ctx.last_window = window; | ||
| if (ctx.last_window != NULL) | ||
| free(ctx.last_window); | ||
| ctx.last_window = strdup(window); |
There was a problem hiding this comment.
strdup is available since C23, but we declare C99 in meson.build.
dmitry-j-mikhin
force-pushed
the
master
branch
from
9457ed7
to
a7a0dac
Compare
It provides constant search and insert complexity. It will be used in subsequent commits to search for window titles, so we can remember layouts for tabs in browser (e.g. firefox)
dmitry-j-mikhin
force-pushed
the
master
branch
from
a7a0dac
to
49320b5
Compare
|
@artemsen thank you for your feedback. I have already corrected most of the comments, except for the comment about cpp) |
| @@ -8,13 +8,13 @@ | |||
| * @param[in] window identifier of currently focused window (container) | |||
| * @return keyboard layout to set, -1 to leave current | |||
| */ | |||
| typedef int (*on_focus)(int window); | |||
| typedef int (*on_focus)(const char* window_key); | |||
There was a problem hiding this comment.
window_key that was created by the function get_cache_key().
I think we should use one one name for this: either const char* cache_key or get_window_key().
There was a problem hiding this comment.
Renamed to get_window_key
Name will identify different tabs in browser (e.g. firefox)
So for now we remember keyboard layout for each tab in browser and reapply it on focus
dmitry-j-mikhin
force-pushed
the
master
branch
from
49320b5
to
aaae26a
Compare
Allows to store keyboard layout for each tab separately. Currently only Firefox and Chromium are supported. Based on Dmitry Mikhin's implementation (PR #6). Co-developed-by: Dmitry Mikhin <dmikhin@webmonitorx.ru> Signed-off-by: Artem Senichev <artemsen@gmail.com>
Allows to store keyboard layout for each tab separately. Currently only Firefox and Chromium are supported. Based on Dmitry Mikhin's implementation (PR #6). Co-developed-by: Dmitry Mikhin <dmikhin@webmonitorx.ru> Signed-off-by: Artem Senichev <artemsen@gmail.com>
Allows to store keyboard layout for each tab separately. Currently only Firefox and Chromium are supported. Based on Dmitry Mikhin's implementation (PR #6). Co-developed-by: Dmitry Mikhin <dmikhin@webmonitorx.ru> Signed-off-by: Artem Senichev <artemsen@gmail.com>
|
Implemented at #7 |
@artemsen, first of all, thank you for this amazing project. I've added some additional functionality to allow you to save and restore your keyboard layout for each browser tab. Some values are hard-coded, such as the LRU cache size and the Firefox name. Perhaps all this functionality should be controlled by an additional runtime flag. But it works for me and I want to share it with you. I think this may be useful in some cases.