User-friendly skin selector

[egmont]

Currently there's no user-friendly way to select a skin. You have to go ahead and read docs, edit ~/.mc/ini or pass command line arguments to mc, locate the available skins in your file system and so on. This might prevent simple users from using skins and limit this feature to power users only.

It would be nice if somewhere under F9->Options one could choose a skin. I imagine a drop-down list which is automatically propagated with the available skins (global and user skin directories scanned for .ini files, alphabetically sorted, duplicates removed). Then of course the chosen value would be saved to ~/.mc.ini. Taking effect immediately (without restarting mc) would be even cooler!

[egmont]

I've attached a demo: changed Ctrl+U (originally swap panels) to iterate among a couple of hardcoded skins.

[egmont]

Demo for runtime skin change

[egmont]

alpha1

[egmont]

Version alpha1 added. There are many things that don't work yet. But the UI is there and you can select a skin there and it's applied, so it gives you a feeling how it's going to work.

struct WDialog's "color" field had to be changed to a pointer from an actual copy of the colors, so that updating the config dialog is performed correctly.

I spent some time wondering where to put the config option. The "Skin:" label and the actual skin name would be way too wide to be put next to each other in one column of a two-column dialog, so it'd have to be either two rows, or a separate box spanning both columns. "Configuration" already occupies the whole screen on 80x24. "Layout" could work, it's probably the closest choice, but still the skin is not about layout. The other ones are about something totally different. So I decided to create a new F9 -> Options -> Appearance dialog.

I think it would be nice to add a couple of options to override the skins' Unicode characters. I'd prefer if the user could override single or double lines, up/down arrows of scrolling and such (maybe these could even be moved out of skins so that skins would only define colors, the rest would be config options). These would also be in the same dialog, so eventually it wouldn't be a dialog for just one single option.

[egmont]

beta1

[egmont]

beta1 is in very good state, please give it a try and please provide feedback! :)

I think it's almost ready for inclusion. Hopefully everything works as expected, and errors (failure to load a skin, e.g. due to lack of 256 color support) are also handled reasonably correctly.

Maybe it needs a little bit of code cleanup, and we should make sure we don't leak memory.

[egmont]

beta2

[egmont]

beta2 even lets you choose single vs double lines in the config box.

All the skins were altered to properly define the double line drawing characters. It's a runtime decision whether those are actually used or not.

The "darkfar" skin is removed, since this was the double-line version of "dark".

"double-lines" was also removed, it's quite like the default with double lines, except for the (IMO very ugly) dark blue menu/button lines. If you prefer to keep this skin, it should be renamed to something else.

"featured" could probably also be removed, it's almost like "default", but uses UTF-8 instead of ASCII compatibility stuff, which is nice.

Maybe the whole [Lines] section could be removed from the skins and be hardcoded in mc, I see no reason why anyone would want to redefine them - any opinions?

[egmont]

beta3

[egmont]

beta3 adds one more featured. So far all skins used a different color for the hotkey. This is against the common UI approach of using underline. My sand256 skin was the only one doing so (also demoing the then new feature of having underlined attribute).

I've added one more checkbox which lets you choose if you'd prefer the color from the skin, or make the hotkeys underlined (and same color of non-hotkey characters). I've also changed the hotkeys in sand256 to use color instead of underline, so you get two possibilities here just as in any other skins.

Technical detail: This required some special hacking, since from now on we use color+attr combos that are not explicitly mentioned in the skin file. I decided to add both kinds of hotkey colors (the special color, as well as the normal color underlined) to the color hashtable, so we can toggle the hotkey look without re-reading the skin and regenerating the color palette. I also thought that this should be private business belonging to the skin engine, rather than branching on the setting all over the code. So I put it in the caching layer, the cached table at the corresponding indexes is populated with either the actual different color, or with the underlined version, depending on the setting. Toggling the setting leaves the hash table intact, requires only the color cache to be regenerated.

At this moment I'm not planning to continue the work until I receive feedback. The patch is in very mature state, I think it only misses the documentation which I will do if you are supportive of getting this patch mainstream. Please try it, please give feedback!!!

Future plans include removing the [Lines] seciton from skin files, and maybe moving a few more Unicode characters from the skins to the config, but I'd like to hear opinions first.

[andrew_b]

Actually, you've partially buried the idea of skinned interface. Really. The idea of interface decoration using skins is define the look'n'feel in the skin as much as possible. The core code should be as simple and minimal as possible, and skin is a place of creativity. Ideally, core shouldn't know anything about how decoration looks. Move of double/single lines, underlined hotkeys, something else explicit skin elements from skin to core contradicts this idea. Personally I dislike this. Sorry.

[egmont]

I also had similar concerns than yours, but then I decided to go this way for the following reasons:

• The current approach doesn't remove anything, in fact it multiplies the number of available skins by (almost) 4, while you can still get all the old ones.

• I expect way more users to find that dialog and try a couple of skins, than to actually go ahead and create their own. So my priority was to make it better for simple users.

• Way back when I created my skin, I chose my preference everywhere but I would have preferred just to design the colors, and let the users choose single/double lines according to their preference. These are really orthogonal, why not conveniently make it so for users?

• Single/double lines is not just a matter of personal taste, it's also a fix/workaround. E.g. the "modarcon*" skins that were designed for the console use double lines, but I don't have double lines on my console, only single ones. With these new features I as an end user can use these skins in the console without having to edit config files.

• As for the hotkeys, I believe that colors introduce unnecessary visual clutter (just too many colors) whereas seeing underlines is more relaxing. I went in the direction (as I did in several other tickets) to make MC follow the standard UI trends that settled down in the last few decades. Underlined hotkeys is one of them. I'm not forcing this on anyone, just offering as a convenient new option for all existing skins, without having to edit them.

• For skin authors so far I have kept the same room for creativity. Although I'm wondering, if nobody has ever come up with a skin that uses anything other than single or double lines, is it worth it keeping complete freedom here?

Indeed I have detached the look of some characters from the color scheme, but I don't see it as a bad thing, actually I see it useful for users, it gives them more possibilities without removing anything. But if you're really against this direction then please check my beta1 patch which contains a skin chooser only. If you like that, you could already apply that patch and then we can continue the discussion.

[egmont]

I actually have one more thing in my mind where I'd like to go ahead in the direction I started. This would add one more checkbox for some of the arrow-like characters.

The default skin, and many other skins use ' and , for arrows, and dot for the "show hidden files" option in either state; these are clearly not because of cool design but because of compatibility. While an arrow is quite self-explanatory, probably many users don't have an idea what that ' stands for. Now as it turns out, my console can display the actual fancy Unicode arrows and circles. (Apparently its font has many glyphs, just the double lines are missing.)

Again, I as an end user would like to choose my favorite color scheme, and independently from that, choose whether the arrows come from that skin, or are replaced by ascii for compatibility. So I'm planning to create one checkbox for this, and make all skins contain a nice arrow.

The exact look of the arrow, circle etc. (there are many Unicode symbols that can be used as arrow) would still be defined in skins.

[andrew_b]

Replying to egmont:

But if you're really against this direction then please check my beta1 patch which contains a skin chooser only. If you like that, you could already apply that patch and then we can continue the discussion.

Hi!

I'd like offer you to change this dialog layout.

• Use "Directory hotlist" as reference design, where
  • listbox with some items is already filled and visible when dialog is shown. So we don't need extra key press to show the list of available stuff;
  • text with actual path to current item.
• listbox with available skins should be filled with value of "description" key not file name of skin;
• select the current skin in the listbox when dialog is shown;
• display an actual path and file name of skin that is being selected in the listbox.

[egmont]

Hi Andrew,

Thanks for your feedback!

• Showing the description: I agree this would be nice (((but I don't think it would be useful. I don't think the user cares about the description of the skin, probably they don't care about the name either. The typical use case I believe is for the user to try them all and pick one whose look they like, no matter what it's called.))) It would require reading and parsing all the skin files (without applying them) when the dialog is open. This would require some nontrivial code refactoring around mc_skin_init().

• Design: I mostly followed the design of Display bits (the only place where you choose from a list under Options) and Advanced chown. I see both pros and cons in Directory hotlist's design. IMO the overall layout doesn't fit the rest of the Options so nicely and wouldn't look that great with additional checkboxes. Saving a keypress is a (really minor) plus, indeed. (((Although I'm not sure how that list should work. Would walking up/down in the list immediately update the skin? Or would pressing Enter update? Or would you need to Tab to a button, press that, and Shift-Tab back to the list?)))

• Showing the filename: Doable I guess, with some additional coding, but again, I don't see any practical use.

• Select the current skin when opening: Obviously, this is also done in the current version (maybe not in beta1).

Please be aware that I don't have too much time to work on MC, and I'd really like to spend that time wisely. Making a cool but hidden feature easily discoverable was a good use of my time. Multiplying the number of skins by 4 by adding the two checkboxes (so that users can choose from a broader variety) was a good use too, and doubling again by addressing comment:12 would be useful too IMO, I'm a bit sad that you don't like it. It would make MC better for the users.

Changing to a totally different UI just to save one keypress, or heavily refactoring to be able to show skin details that probably noone would care about, sorry, these really don't sound like a good use of my time. These would require a day or two, maybe three from me, just to let the user achieve the very same goal in a slightly different way.

If you point out code styling problems, if you have minor layout issues with the current solution etc., I'm of course happy to do it. If you decide to choose beta1's features only, let me know and I'll backport bugfixes (if any) from beta3. If anyone, either you or someone else, either right now or after a couple of mc releases decides to redo the UI to something completely different, I don't mind at all. Please understand that after spending 3 days adding a feature that didn't exist at all, you ask me to spend 2 more days on saving one keypress and showing some additional useless info, I have to say sorry but I would really like to spend my time on much more useful things. Thanks for your understanding :)

[vitalif]

Hi everyone. Yesterday I've thought of the same idea (skin selection UI)... and actually implemented it without reading the bugtracker first :-[...

Sorry if I'll mess up something by this... But I've already done it, so I'll post my implementation here... It's a simple listbox invoked from "Options->Skin" menu item :-)

[vitalif]

My version of skin selection UI... :)

[vitalif]

/* Please don't be surprised when you'll look at the attachment, I've just modified it, taking your skin listing code because I think it's better than mine (I originally used GList and glob() instead of simpler GArray and readdir()) */

[vitalif]

About the single/double lines configuration and etc: I also think it would be better not in the core. But it still may be useful as a part of "skin configuration" UI, i.e. for example if you want double lines, you set double lines in that UI, it modifies the skin and saves a modified version in your personal directory. Such approach won't break skinning :)

[egmont]

Hi vitalif,

My opinion after a quick glimpse:

• It's a pity you didn't look for this feature first, I hope next time you'll save yourself some precious time :)

• andrew_b had a concern that my version required 1 extra keypress compared to what he had in mind. Your version requires even more keypresses (assuming that you don't know which skin you want to change to, but rather you'd like to take a loot at all of them - my version: Enter, down, Enter, <repeat> that's 3 keypresses to take a look at a skin, your version: F9, O, S, down, Enter, <repeat> that's 5 keypresses in a much less intuitive order for trying out each skin).

• I can't see yet how your UI would accomodate extra fields for double lines or such and how the navigation keys (up, down, tab, enter etc.) would work in that case - that's the same issue I had with andrew_b's suggestion.

• If you change skin a couple of times, you'll see that some colors (mostly directories / normal files) are not updated correctly, e.g. colors of another skin appear there and such. I took care not only to make this work perfectly, but also to free up the allocated slang or ncurses color pairs so we don't run out of them even after zillions of skin changes. So I believe that for the backend part my code should be used. (It was quite complicated to get it right, and took me more than a day.)

• I love the way you move the Default skin to the top with a special label, this should be backported to my work.

• You say "you set double lines in that UI, it modifies the skin and saves a modified version in your personal directory" -- read a skin, modify it, write it out in order to read it back and then use it??? It's extremely overcomplicated whereas I provide a simple straightforward solution. Let alone the fact that we pollute the user's directory, and those autogenerated skins won't catch up with subsequent mainstream changes.

[vitalif]

Oops, sorry. No segfault, all OK.

[vitalif]

Patch with removed double line / underline hotkeys settings

[vitalif]

I like your version - selecting skins with less clicks is more handy. :) I don't think it needs further reduction of click count.

I would like to see it in near release of mc, i.e. I'm voting for this feature :) and I've removed double line / underline hotkeys selection from your patch - maybe this simpler version can get merged faster?

Personally I don't even think many people care about double lines (but I'm sure everyone wants to be able to change skins!) - so maybe the extra settings can just be reviewed later as a separate patch?

[egmont]

I'd be totally fine with applying just the basic skin selector first, and then keep the additional features (double lines, underlined hotkeys, ascii vs unicode symbols (to be implmented) and your special <Default> label) open for future discussions/review.

The two main reasons I kinda care about double lines are:

• I've found a great skin where I love the colors, but IMO double lines look very ugly (with the terminal+font I'm using)
• The modarcon* skins were designed for the linux console, but use double lines whereas for me only single lines work there.

So it's both a feature as well as a workaround for terminals with limited capabilities.

Unfortunately nowadays the bottleneck seems to be the human resource (time) by main mc developers (e.g. stalled conversation over here), but I hope we'll hear constructive feedback and see the work appear in next mc.

[vitalif]

P.S: Initially I've also had a thought about including skin setting to "Layout" dialog box (because in russian it sounds "Внешний вид" which is more like "Appearance").

[andrew_b]

Thanks for patches!

I used the mc-skin-selector-beta3_nolines.patch​ patch as base. My modifications are following:

• use GPtrArray instead of GArray to store skin list;
• use GDir instead of dirent to scan skin directories.
• apply mc code indentation rules;
• add new action to the keymap file;
• add trivial help topic for the new menu item.

Branch: 2165_skin_selector
Initial changeset:92dfe6a8429c695b82e5ecc39783aa7d9779fd53

Please review.

[egmont]

Looks good, thanks.

One minor leftover issue: F1 brings up the help page describing where the skins are located and how to create one. I'm not sure if it's useful/relevant here and is too technical at this point, I'm wondering if maybe we should rather bring up a very short new help page just saying that "here you can choose the skin". (If we'll add additional options later, e.g. double lines or underline, those would also be described there.)

[egmont]

doc fix suggestion

[egmont]

I've attached my suggestion for updating the documentation. (Russian translation to be synced up...) Sorry, I should've done it at the first place :)

The user is given a link to technical details about skins, but it's not immediately shown since at this point probably most users don't really care about it. I also followed my generic approach of using the word "Appearance", keeping in mind that later on I'd like to put a few more options here, not just the Skin. Whether you agree with this or not will be subject to further discussions, but at least I think the current situation is now consistent with itself.

[egmont]

special label for the default skin

[egmont]

I've also backported Vitaliy's idea of handling the default skin specially. It is moved to the top of the list (and removed from the regular listing, as opposed to his approach where it was duplicated), and it receives a translatable label.

The code that builds up the list could be simpler (could avoid duplication) if the selected entry would be maintained even when prepending new entries. Then adding the default skin would also be handled in the "for" loop, just with a different flag so that it's inserted at the beginning of the list. But I'm afraid to touch the listbox widget on which many others depend, so I went for building up the list in its final order: add the "default" skin first, and then add the rest in the loop.

[andrew_b]

Replying to egmont:

I've attached my suggestion for updating the documentation. (Russian translation to be synced up...) Sorry, I should've done it at the first place :)

Applied. Thanks!

[andrew_b]

Replying to egmont:

I've also backported Vitaliy's idea of handling the default skin specially. It is moved to the top of the list (and removed from the regular listing, as opposed to his approach where it was duplicated), and it receives a translatable label.

Applied too.

[egmont]

Cool :)

Pls don't forget to recreate the .po files again (for the "< Default >" string).

Once merged in master, I'm planning to open another ticket to discuss additional options that might one day go into the Appearance dialog - and let this ticket be closed.

[andrew_b]

Merged to master: [69b5cf5a818b594dc1c3e45b134fcbf8d55d6377].

git log --pretty=oneline 25bc34a..69b5cf5

[egmont]

Continued in #3169.