Philantrop / calibre-marvin-manager

A calibre User Interface Action plugin supporting Marvin
6 stars 6 forks source link

Enhancement: Add custom column mapping for 'Reading list' and 'Read' flags #22

Closed GRiker closed 10 years ago

GRiker commented 10 years ago

MXD currently allows the user to map certain Marvin metadata to calibre custom columns: Annotations, Collections, Last read, Progress and Word count.

This enhancement would add support for two of three Marvin's special flags, Reading list and Read. New is not included, because it is a state that is only meaningful within Marvin.

Flag synchronization would occur in one of three ways in MXD:

Ideally, if we knew where the flag was most recently updated (calibre or Marvin), we could assume that would be the sync master. Calibre does track metadata updates in its database, Marvin (as of 2.4) does not.

Reading list logic

If a calibre custom column mapping exists for Reading list, when flag synchronization is initiated, the value in the mapped calibre custom column would be applied to Marvin. In other words, calibre is the sync master for Reading list.
NB: This means that if a user adds a book to their Reading list in Marvin, that assignment would be overridden during flag syncing. The assumption is that if the user has gone to the trouble of creating a custom column mapping for Reading list in calibre, that is where they are managing their reading list.
Clearing the Reading list flag in the Flags column context menu would clear the flag in both calibre and Marvin.

Read logic

If a custom column mapping exists for Read, when flag synchronization is initiated, if either calibre or Marvin has the Read value set, then the other flag would be set to match. For example, if the user sets the Read flag in Marvin, the next time they run MXD and initiate flag syncing, the Read flag would be set in the calibre custom column. And, if the user sets the Read flag in calibre, the next time MXD is run and flag syncing is initiated, the Read flag would be set in Marvin.
The assumption is that Read is a universal state for the user per book.
NB: If a user wishes to clear the Read flag, they would need to do it manually in both calibre and Marvin, as clearing it one location only would be overridden during the next flag syncing operation. Using the Flags column context menu to clear the Read flag would clear the flag in both calibre and Marvin.

Removing a book from Reading list when a book is marked Read

If a book's Read status is set to True, it would be removed from Reading list at the same time.
NB: This feature is not going to be implemented, as it would lead to user confusion. I'm leaving the description in place for historical purposes.

RosieH commented 10 years ago

Hi G As we've discussed, I'm not sure automatically removing a book from the reading list when it is marked read is the correct logic. I re-read books all the time, and when i think of a book i want to re-read, I add it to my reading list. With the above logic I would have to manually remove the read flag from both Marvin and calibre before I could add the book to the reading list, to avoid it being automatically removed from the list on the next sync. I don't know that I would always remember to do this.

Would it be worth keeping the logic, but putting up a message to the user on sync if books are about to be removed from the reading list? An "are you sure" button so the user could over-ride the removal if they wanted to.

Thoughts?

GRiker commented 10 years ago

With respect to the "Removing a book from Reading list when a book is marked Read" feature, I think I'd rather not add the feature. If users have to work around a "feature", or need to be warned when using it, it doesn't seem like a desirable addition.

Philantrop commented 10 years ago

I like the first two ideas but I'd implement them differently:

Reading list logic: I'd suggest not to define a sync master. Instead, if the state between Calibre and Marvin doesn't match, check for the latest modification time in both and use the state from the more recently modified source.

Read logic: Having to clear the "Read" flag manually somewhat defeats the purpose of synching. Thus: Same suggestion as before.

Removing a book from Reading list when a book is marked Read: Marvin doesn't remove a book from the "Reading list" if you mark it read. I think you should really do the same for consistency's sake and to stick with the principle of least surprise. Furthermore, this would be too much "magic" and interfere with users' workflows (cf. RosieH's comment) without any great benefit.

Philantrop commented 10 years ago

Btw, it would be lovely if you allowed not only for mapping to (custom) columns (which are way over-used in Calibre) but to custom tags as well. I don't like adding a gazillion custom columns (because they would show up in my OPDS feed) but use tags for managing my reading list status.

GRiker commented 10 years ago

Wulf, my strong preference would be to use the logic you propose, but I don't know of any way to determine the last time a particular flag was modified, either in calibre or Marvin - it's simply not tracked, other than at the coarsest level, the last time the overall db was modified. Additionally, I would need to know the latest flags mod time on a per-book basis. Do you have some other idea about how to detect this?

I agree with the comments about updating Reading list when Read is set. I will update the first post to reflect that I'm not planning to implement this.

WRT to custom tag mapping, I will look at what would be involved. Are you thinking of mapping tags for Reading list only? It wouldn't make sense with most of the other mappings.

Philantrop commented 10 years ago

In its "books" table, Calibre has a high-resolution "last_modified" timestamp for each book record. IIRC from the last time rummaging through Marvin's sqlite database, it features a similar timestamp, too. Of course, you can't track individual tags or columns but the respective book records should be good enough, I think.

Custom tag mapping: I was thinking of both reading list and read tags. Everything else, I agree, doesn't make much sense, indeed.

GRiker commented 10 years ago

That would be great if there's sufficient timestamp detail in the respective dbs. I will research.

GRiker commented 10 years ago

Kris says there's no per-book timestamp for metadata updates.

Philantrop commented 10 years ago

Oh, yes, I just checked again and surprisingly ;), he's right. I confused it with another field. Well, let's make him add one. ;-) Cf. email.

kguil commented 10 years ago

I'm assuming the "reading list" flag issue will be resolved once there is a Last Updated timestamp in Marvin's [Books] table, right?

If this is the case, I'll schedule it soon. It seems that sqlite doesn't have a special timestamp column type that is automatically populated with the time at update/insert and I'll have to manually add this for every operation that updates/inserts a row in [Books].

GRiker commented 10 years ago

I have a provisional build of MXD with intelligent syncing logic for the Read and Reading flags. While we're waiting for Kris to add a LastModified column to mainDb:Books, I'm using DateOpened as a surrogate. DateOpened can be refreshed in Marvin by opening the book.

Here's my proposal for the behavior of the new LastModified field that we're asking Kris to add:

The provisional build can be downloaded here.

Here's a plain-english summary of how the sync logic currently works: There are three ways to invoke synchronization:

When syncing, the timestamp reflecting the last metadata modification date is compared. For calibre, this comes from the metadata objects .last_modified property. This value is updated when the user makes any change to the book's metadata. For Marvin, this value will come from the LastModified field. (For now, it's coming from DateOpened.) The behavior of the Flags column context menu options has changed as well. If there is a custom column mapping for Read or Reading list, the flag update will be applied to both calibre and Marvin.

In order to make calibre the sync master for a given book, you would change the flag of interest (Read or Reading list) in the mapped custom column. Note that ANY change in calibre's metadata will update its timestamp. In order to make Marvin the sync master for a given book, you would change the flag of interest, AND open the book in Marvin. This will update the DateOpened field, which is being used for the comparison. Once Kris adds the new LastModified field, this won't be necessary.

When syncing, the most recently updated environment (calibre or Marvin) becomes the sync master, and its current value is applied to the other environment.

It is entirely possible that there are some scenarios where a sync master may be inferred that is not the user's intent. The rule of thumb will be "wherever the last metadata change was made will be the sync master".

kguil commented 10 years ago

Agreed. I'm combining this update with a new metadata field called "Notes" where a used can add arbitrary notes for a book.

2.4 will be in the app store late next week. Very soon after that (2.4.5 or 2.5) I'll be releasing an already completed update with support for a new cloud service. I'll schedule the change for immediately after that. Expect betas after the 27th (I'll be in London from the 21st till 27th).

RosieH commented 10 years ago

Hi G

Just to let you know I haven’t vanished off the face of the earth, rather I have been really busy and unable to do the testing I wanted on MXD. I have done a little ad hoc testing of the new functionality but I keep getting called away to other things. I’m trying to carve out some time to do a formal test of the things that didn’t seem to be working right over the next couple of days, so I can send you a debug log.

Just for your info (and I’ll provide more detailed descriptions/logs soon), one thing that happened is that the final MXD info box (something about dropping un-needed cache entries) stayed on the screen even though MXD had come back with its list (and I could still work with MXD but the info dialog box stayed in the foreground of the screen the whole time). Plus I had updated some of the flag fields in calibre but couldn’t get the Marvin ones to change, whatever method I tried. But I didn’t have much time so I need to sort out exactly under what circumstances I’m having the problems before submitting the actual bug reports.

Cheers

Rosie

GRiker commented 10 years ago

Rosie, If you're seeing an info box left stranded on-screen, there must be an associated error in the debug log. I have never seen that behavior in released versions of the plugin, but I don't doubt that you're experiencing it. The next time you're working with MXD, please run in debug mode so that when something unexpected happens you already have the debug trace available.

GRiker commented 10 years ago

This feature has been implemented. It is available with the latest Marvin beta (2.6.661). You will need to download and install the latest iOSRA plugin and the latest MXD plugin.

WyndhamMisio commented 10 years ago

Small observation: Perhaps these two plugins are getting slightly larger, as it seems they are taking a little longer to install. The problem here is that there appears to be nothing happening (initially) after clicking to install...

Some sort of installation progress indicator would be helpful.

GRiker commented 10 years ago

A valid observation, but plugin installation is managed by calibre, not by the individual plugin, so that's beyond my control. I do know that if you have an iDevice connected to your computer when updating the plugins, it takes longer, so perhaps disconnecting your iDevice would speed things up.