kindlepdfviewer icon indicating copy to clipboard operation
kindlepdfviewer copied to clipboard

Need some configuration methods for different configurations in future

Open dracodoc opened this issue 11 years ago • 23 comments

At the earlier stage the possible user preferred configurations are not much and simple, so it's acceptable for user to directly edit lua files to change some settings, like global refresh. They may not be preferred by every user, so it cannot be applied to builds directly.

However, there have been some more preferred changes on certain configurations now, it's not easy to apply them manually for every new build. I generated a patch file with mercurial for all the changes I wanted, but the patch process still need manual verification.

Maybe you can consider some configuration methods in future to incorporate these changes? It's not a high priority issue but better to have it.

I'll list the changes I applied here for your reference:

  1. some changes are minor changes, just parameter value change, like global refresh, pdf margin color, popup dialog box time delay, these can be put in a config file easily. Actually I think there was some font config file in certain NuPogodi patch, but it was not used later.
  2. some changes involves different key bindings. I asked houqp to implemented a bookmark jump feature, which would use the shift+ <> keys, so I deleted the zoom grade command for this key in one place, moved the default font zooming key to alt + <> in another place before adding this feature. This change is more complicated, maybe have to wait the user customizing key binding is available.

I don't know how to upload a file here, so I just uploaded the patch file I made there: https://www.box.com/s/1g3rabvm24juia2e9hea

dracodoc avatar Sep 09 '12 16:09 dracodoc

I had a look at your unireader.patch; I agree that 2000ms for those bookmark messages is far too long. However, your suggested value of 50ms is too little. In my current pull request #263 I have reduced the value to one and a half second, which is a bit better than two seconds, but long enough to be noticed by the user (50ms wouldn't be noticed).

tigran123 avatar Sep 09 '12 18:09 tigran123

The actual value is not important, in my case the 50ms setting will not make the whole dialog disappear in 50ms, the dx refresh rate is simply not that fast, so it is probably same as 300 ms, but I didn't bother to change it. I can image a K3 user could have a different result, and there probably would be other users that doesn't mind to have a little longer delay time.

The patch file is meant to summarize several points that can be parametrized. I'm not suggesting to incorporate them into official build directly.

dracodoc avatar Sep 09 '12 18:09 dracodoc

It seemed that there used to be some configuration file, according to the wiki https://github.com/hwhw/kindlepdfviewer/wiki/Userguide

There used to be .reader.kpdfview.lua, then I believe it changed to settings.reader.lua, then it was eliminated all together.

dracodoc avatar Sep 17 '12 19:09 dracodoc

It is not eliminated, it still exists and is in fact the very first thing that the reader loads on startup.

tigran123 avatar Sep 17 '12 19:09 tigran123

Sorry I didn't make my point clear. settings.reader.lua still exist but it was generated by kpv, right? So this file is not supposed to be edited by user to customize parameters, right?

dracodoc avatar Sep 17 '12 20:09 dracodoc

One of the values in the file (namely the fontmap table) is accessible to the user from within KPV. The other two values are either constant (version) or automatically updated to point to the last file opened (lastfile).

tigran123 avatar Sep 17 '12 20:09 tigran123

By "customize parameters", I mean user can download a new build, edit the configuration file or just copy his old configuration file to keep his favorite settings, like these entries list in wiki before: ["partial_refresh_count"] = 5, ["pan_overlap_vertical"] = 30 and even more like the patch file I posted.

The point of configuration file is to collect most possible user customizations in a center place, so user can easily manage them and keep them between versions. These kind of customization is not the user setting from within kpv.

dracodoc avatar Sep 17 '12 20:09 dracodoc

Currently, many of the parameters are specific to a book and so they are stored in the history files for each book. The generic settings are stored in settings.reader.lua. To keep the favourite settings the user has to keep the content of his history directory and also the file settings.reader.lua. So, I still didn't understand what part of the existing functionality you consider insufficient and precisely in what way it is insufficient.

tigran123 avatar Sep 17 '12 20:09 tigran123

All the settings I'm talking are global settings, they are not likely specific to a book. Like these: global refresh pdf margin color popup dialog box time delay

Actually they can only be modified by user manually editing lua files, which are spread out in 2 lua files in multiple places, and have to be done for every new build.

dracodoc avatar Sep 17 '12 20:09 dracodoc

Is .reader.kpdfview.lua is affecting the behavior of your kpdfview? I think it is already replaced with settings.reader.lua. So there is only one setting file.

houqp avatar Sep 17 '12 23:09 houqp

Yes, it's also my findings. Modifying current configuration file will not achieve all the result I want, so I've been manually modifying lua files for each build, just like the patch file I posted above.

dracodoc avatar Sep 17 '12 23:09 dracodoc

Still dont quite get what you mean. ;P

So you are saying that modifying settings.reader.lua does not work for some of the settings?

houqp avatar Sep 18 '12 00:09 houqp

You can check the patch file I posted on first post in this issue. I modified several places in unireader.lua and creadler.lua, which are not covered in settings.reader.lua.

dracodoc avatar Sep 18 '12 01:09 dracodoc

OK, I think I get what you mean. You are suggesting adding more configurable entries in settings.reader.lua, right?

houqp avatar Sep 18 '12 01:09 houqp

Yes! And I also want the bookmark jumping feature you wrote per my request, but that is not a simple parameter value so it'll be more difficult to config. Maybe with a switch to turn it on or off?

Sorry just closed the issue by mistake

dracodoc avatar Sep 18 '12 02:09 dracodoc

What is a bookmark jumping feature?

tigran123 avatar Sep 18 '12 13:09 tigran123

Basically I make bookmark in places I would have note when I read a book. After I finished the book, I'll review each bookmark position and make note in pc. So I need to be able to jump from one bookmark position to next bookmark position directly, and it don't have to start from a bookmark page, just a shift+page down will bring me to next bookmarked page.

It's a feature I used very often but will have key binding conflict with current functions, so when houqp wrote this feature and sent to me to test, I have to replace some other command that I don't use. I thought there is no good way to incorporate this feature into main build without removing other command, so I was waiting for the new UI before suggesting merge it, since new UI will have customized key binding.

Though I just realized it's still possible to add it now, if we use a switch parameter in configuration file to turn it on or off.

dracodoc avatar Sep 18 '12 13:09 dracodoc

Ok, I cannot promise, but I will see if I can managed to get some free time working on it this weekend.

houqp avatar Sep 19 '12 03:09 houqp

Thanks houqp, take you time!

dracodoc avatar Sep 19 '12 03:09 dracodoc

popup dialog box time delay --- Actually they can only be modified by user manually editing lua files, which are spread out in 2 lua files in multiple places

One can change the only parameter msec in the only function showInfoMsgWithDelay(text, msec, refresh_mode) in dialog.lua. Inspecting this code could also help you to understand that no need to do it at all - simple pressing 'anykey' closes the help window

Two suggestions able to solve the problem:

  1. introduce the global parameter read_speed (reasonable values 50-100ms/character) and define the default msec from the text length - say, 500+read_speed*string.len(text)
  2. one can also add some kind of general parameter 'verbosity' that would define whether kpdfviewer shows uncritical warnings & help messages or not (for experienced users).

ghost avatar Sep 20 '12 18:09 ghost

I was not referring time delay itself are spread out, it's all the 4,5 changes in different aspects that are spread out in multiple places.

dracodoc avatar Sep 20 '12 18:09 dracodoc

@NuPogodi Thanks for the tip about pressing any key to close the info window.

tigran123 avatar Sep 20 '12 18:09 tigran123

@houqp and @tigran123 has helped to add the bookmark jumping feature to master build, and the global fresh is already default, so the remaining changes I requested should be very easy to be configurable now.

  1. Default font for epub. There could be a option to assign default font in the epub font selection screen, once it was set, the information should be saved in reader.settings.lua
  2. The color of pdf margin. This is also very easy to change or make it configurable.

All the configurable settings should be written to the setting file, so user can just install the new build over the old build and keep the setting file to keep his settings.

dracodoc avatar Oct 15 '12 13:10 dracodoc