Skip to content

Add some settings to Display mode/Other settings menu#3129

Merged
Frenzie merged 1 commit intokoreader:masterfrom
poire-z:fm-settings
Aug 25, 2017
Merged

Add some settings to Display mode/Other settings menu#3129
Frenzie merged 1 commit intokoreader:masterfrom
poire-z:fm-settings

Conversation

@poire-z
Copy link
Copy Markdown
Contributor

@poire-z poire-z commented Aug 24, 2017

Some hidden file manager settings may have a place in menu, and they may fit in the newly added Display mode/Other settings menu (from coverbrowser plugin).

othersettings

From #2589 (comment) :

show_file_in_bold filemanager items in bold : true/false/"opened" #2457
home_dir_display_name used by filemanagerutil.abbreviate(path) to shorten directory in title bar made as a toggle (true => "~", false => nil) (people who wants something other than ~ may set it manually as before
autoremove_deleted_items_from_history #2583 (comment)

This menu comes from coverbrowser plugin, so it may look strange to deal with core settings there, but it fits the menu hierarchy.
The first one has a real place there. It's not too obvious for the other 2, but better to have them burried there that not available, no ?

Was tempted to add there auto_book_status (show book status onEndOfBook) but it feels really out of place as it's about reader. No obvious place for it on Reader side, it does not deserve a first-level item, may be in a "Other settings" subitem ?

(separator=true moved just for logical reason: as it's displayed after an item, better for it to be at bottom of item definition)

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 24, 2017

Did you consider using a menu sort order so that the plugin part will automatically be left out without the plugin?

@poire-z
Copy link
Copy Markdown
Contributor Author

poire-z commented Aug 24, 2017

No, I didn't. You're thinking about making these available also for android devices where coverbrowser is disabled?
Not sure how to do that, how would you do that ? (while keeping the other items self contained in the plugin, without leaking their ids into core ?)

edit: Not involving menu sorter, it could be easier to, even for android, registerToMainMenu() in CoverBrowser:init(), and in CoverBrowser:addToMainMenu(), build a different menu if android / others.
But the same "Display mode > Other Settings > these 3 new settings" would be a little awkward. How would you name it for android: "Display settings > these 3 only settings" ?

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 24, 2017

You'd add another entry in the menu_order definition:

    filemanager_display_mode = {
        "plugin_stuff",
        "more_plugin_stuff",
        "----------------------------",
        -- the above will simply be ignored by MenuSorter if plugin_stuff is nil
        "core_stuff",
        "more_core_stuff",
    },

I spent most of my time on MenuSorter making sure it could deal with such situations without breaking (except I never thought to throw the separator thing at it). Most submenus are done the traditional way but in principle many of them could be migrated to the MenuSorter system, even if only internally.

That is, you could define your own menu using the MenuSorter method while still keeping it out of the main loop.

plugin_menu_order = {} -- you know how this works
plugin_menu.whatever = {text="etc"}
menu_items.my_plugin_name = MenuSorter:sort(plugin_menu, plugin_menu_order)

In fact that might be preferable to keep plugin configuration separate. On the flip side, in that case you can't reorganize the entire menu at your personal whim.

Anyway, those are just some possibilities I explicitly designed in there but which aren't currently used much if at all. :-)

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 24, 2017

edit: Not involving menu sorter, it could be easier to, even for android, registerToMainMenu() in CoverBrowser:init(), and in CoverBrowser:addToMainMenu(), build a different menu if android / others.
But the same "Display mode > Other Settings > these 3 new settings" would be a little awkward. How would you name it for android: "Display settings > these 3 only settings" ?

Something like "filemanager (display) settings"?

@poire-z poire-z force-pushed the fm-settings branch 2 times, most recently from 1a6ea87 to 2f215db Compare August 25, 2017 07:02
@poire-z
Copy link
Copy Markdown
Contributor Author

poire-z commented Aug 25, 2017

OK, made that menu with new 3 settings at 1st level named File browser settings on Android.
(I did not make menu sorter involved, as I somehow prefer the whole indented table definition for a better feel of menu hierarchy.)

It still itches me a bit that this core settings menu is defined here in a plugin. They could probably be in filemanagermenu.lua, (as self.menu_items.filemanager_settings), with both "filemanager_display_mode" and "filemanager_settings" in elements/filemanager_menu_order.lua, and this plugin could steal them from menu_item in addToMainMenu(menu_items) and unset menu_items.filemanager_settings = nil so main menu sorter ignores it.
Should I try that ? or would the current stuff do for now ?

No rewording suggestions ?

separator = true,
},
{
text = _("Abbreviate home directory with ~"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about Shorten home directory to ~?

end,
},
{
text = _("List with image and metadata"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List with images and metadata (or maybe Detailed list with images/thumbnails)

end,
},
{
text = _("List with metadata, no image"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, plural sounds better

end,
},
{
text = _("List with image and filename"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and finally images and filenames

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 25, 2017

As you wish. :-P

@poire-z
Copy link
Copy Markdown
Contributor Author

poire-z commented Aug 25, 2017

Tried to put them into filemanagermenu.lua, but plugins/registered_widgets are added eartly to menu, before menu's own stuff, so in this plugin's addToMainMenu(menu_items) , the menu_items it gets does not yet has the keys set in filemanagermenu.lua... so I can not grab them to move them into Display mode. And it would pain me a bit to have 2 submenus on others than Android.
It would need some work on filemanagermenu.lua (may be just moving the registered_widget additions from early to late), but i'm not confident with changing things there that work :)
So, safer to let them in this plugin.

Updated wordings (including those from original coverbrowser not noticed weeks ago :).
(btw: is plural in "no images" ok in english ? I'm used to writing "aucune image" in singular in french).

(The "file changed" diff is a bit ugly, because I gain one indentation level by using if self.ui.view then return end)

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 25, 2017

(btw: is plural in "no images" ok in english ? I'm used to writing "aucune image" in singular in french).

That would depend on context. When in doubt you can always check on one of those sites like Linguee or Bab.la. They've usually got fairly decently translated stuff.

http://www.linguee.fr/francais-anglais/search?source=auto&query=aucune+image

For example, you've got this pair:

  • Sans aucune image sur les écrans, c'est difficile de voir à quel point la console peut être attractive.
    nintendo.fr
  • Without any images on the screens, it's difficult to communicate the full appeal of the console.
    nintendo.ru

(… nintendo.ru? Odd to get English from there, but whatever)

I think that in English zero probably always takes a plural form, unless it's a mass noun like sand or water. Of course you could always say that you don't have even a single image when you have pas d'images. :-P

Speaking of which, what's the difference between J'ai aucune image and Je n'ai pas d'images? To me the first sounds more forceful.

@poire-z
Copy link
Copy Markdown
Contributor Author

poire-z commented Aug 25, 2017

OK, so "no images" can stay.

Speaking of which, what's the difference between J'ai aucune image and Je n'ai pas d'images? To me the first sounds more forceful.

No real difference, and you're right about the forceful (and we use the negative in je n'ai aucune image)
I would always use singular with aucune image. Just read that with sans (without), it's singular or plural depending on how it would be avec (with) :) Un homme avec parapluie, un homme sans parapluie. But une chemise avec manches, une chemise sans manches.

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 25, 2017

we use the negative in je n'ai aucune image

It seems to me that even the n' is often dropped outside of writing.. ;-) But yes, that was a silly oversight on my part. Double negatives don't come naturally. I've managed to internalize a few of the most common ones like ne…pas, ne…plus and ne…jamais so they come out without thinking but that's about it.

@Frenzie Frenzie merged commit 2595dbb into koreader:master Aug 25, 2017
@poire-z poire-z deleted the fm-settings branch August 25, 2017 15:28
@poire-z
Copy link
Copy Markdown
Contributor Author

poire-z commented Aug 25, 2017

It seems to me that even the n' is often dropped outside of writing..

Yes indeed, in talking, we easily drop the n', even the ne : j'ai pas de parapluie, je viendrais pas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants