[RFC] hkml_view: highlight current selected line

[ryncsn]

On some terminals, the cursor is dimmed so it's hard to tell which line is the selected line. highlight it by reverse the backgroud color.

Let me know if there are any alternative config or design idea, thanks!

[sjp38]

Hello @ryncsn , thank you for this nice PR!

I think this is a nice idea. Actually we used to color the current line as yellow for a similar purpose, as can be shown [1] from the outdated demo gif. We later removed it with commit 932dc89 ("hkmlview: remove focus color"), though, because it was difficult to distinguish with green lines for diffs on patch mails.

This PR will not have the problem of coloring, as it reverse the background color. I tested this with patch mails and confirmed it works well with diff lines.

Nonetheless, as I'm not good at UX, I'm not very sure if it is the best to always enable. I think a safe option would be making this optional, and later consider making it enabled by default. The mails list view has "set display effects" menu (you can show this after pressing m key) for displaying mails of specific time range differently. By default, the list suggests users to dim old mails that supposed to already been read. What about extending this feature for current selected line? Like the 'dim old' feature, we could always ask users to turn the feature on or off, when starting list.

Implementing this idea may take unnecessarily long time, since hkml code is quite hacky as the name says. And I guess you want to spend more time on kernel, not hkml ;) If you don't mind, I could implement it on top of this PR. What do you think?

[1] https://github.com/sjp38/hackermail#demo

[sjp38]

I prototyped my idea and pushed to a branch (https://github.com/sjp38/hackermail/tree/focus_row_display_effect). `git diff 00db26c..ae83d08" should show what I'm thinking.

So, the prototype asks the question at the beginning of mails displaying. It asks more user inputs, but I believe that's not too much question. In future, if this is a real issue, we could think about supporting config files. Actually I'm planning to work on a such thing for custom shortcut keys setup.

Also the prototype asks the question only at that time. In near future, we could add another mails list menu (the menu that comes up after pressing m key) for changing the decision, like hkml does for dim_old option.

[ryncsn]

I prototyped my idea and pushed to a branch (https://github.com/sjp38/hackermail/tree/focus_row_display_effect). `git diff 00db26c..ae83d08" should show what I'm thinking.

So, the prototype asks the question at the beginning of mails displaying. It asks more user inputs, but I believe that's not too much question. In future, if this is a real issue, we could think about supporting config files. Actually I'm planning to work on a such thing for custom shortcut keys setup.

Also the prototype asks the question only at that time. In near future, we could add another mails list menu (the menu that comes up after pressing m key) for changing the decision, like hkml does for dim_old option.

Looks good to me! Thanks!

Just one suggestion from my side, the asking prompt upon each launch seems getting too long, might be better if we have a config file so hkml can simply launch the session without asking everytime.

[sjp38]

Sounds good, I will merge the branch after cleaning up the code, and set a future work for supporting a config file that can remove the prompt.

[sjp38]

I just merged the branch and implemented a new mail list menu ("set focused row display effect") for changing the effect. Also added a TODO item for future config file support (commit 47c376e TODO: add an item for config file support of focused row display effect). Closing this PR as the needed changes for now are all made.

Please feel free to open issues or PR for bugs in the implementation, or for tracking the config file support progress!

[sjp38]

Hi @ryncsn as I promised, I just pushed a change [1] for the config-based display rule setup. Hopefully the diff is self-explaining the feature. Please feel free to let me know if it is unclear or you want some more changes.

[1] 19d0f84