Register Guidelines E-Books Search Today's Posts Mark Forums Read

Go Back   MobileRead Forums > E-Book Software > Calibre > Development

Notices

Reply
 
Thread Tools Search this Thread
Old 04-10-2012, 09:21 PM   #1
idea00
Junior Member
idea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enough
 
Posts: 7
Karma: 684
Join Date: Apr 2012
Device: android
Improved UI for Search/Replace

Hi,

I'm working on an improved UI for Search/Replace that adds the following capabilities:

- Unlimited search/replace definitions instead of just the fixed three.
- Ability to save/load the definitions for subsequent use.

The code can be found at:
lp:~idea00/calibre/search_replace

but is not yet ready for merge (have a few questions below.)

A screen shot of the UI implementation can be found at:

http://img37.imageshack.us/img37/9321/searchreplace.png


Rationale
The reason I made this change is that I have several badly converted epubs with leftover PML markup (\aXXX characters the full list is in PML Characters) and was converting them manually (unzip, search/replace with a text editor, zip.)

So I thought:
"This is stupid, I should just write a script to do this"
"Wait, this is a problem others may have, maybe I should add this functionality to Calibre"
"How? Where? Lets look at the develop docs."
"Ahh... a plugin!"
"Wait... Wait... Wait... before I write a new plugin lets make sure this has not already been implemented."
"Ok.. Calibre conversion settings... look and feel, heuristic processing... search and replace,..."
"What was that SEARCH AND REPLACE? Calibre has search and replace?!"

Long story short I need more than three search and replace definitions and I'd like to be able to save them so I don't need to input them every time I need to do this conversion (or any other common one.)
Now for the questions:

Backwards Compatibility
I changed the options 'sr1_search', 'sr1_replace', 'sr2_search', 'sr2_replace', 'sr3_search', 'sr3_replace' with a single 'search_replace' option (plumber.py and cli.py). This means any plugins that modifies the old search/replace options will not work, but I found no evidence of a plugin changing the user preferences so I believe this is okay. I think I can make my change backward compatible by introducing srX_search/srX_replace options dynamically based on the number of actual search/replace definitions, but I'd probably need to make substantial (ugly) changes.

Internationalization
I'm guessing the underscore function '_()' used around strings is the hash look-up for the language specific string, but I haven't deciphered this fully so pointers would be welcome. Specifically how are strings in the *.ui files handled and what else needs to be done to add new strings.
I used the help string for "SEARCH AND REPLACE" in 'cli.py' as the help string in the OptionRecomendation for the new search_replace option so that should be okay, but now the old help strings for srX_search/srX_replace are just laying around unused so are there any tools for cleaning up unused strings or should this be done manually (and how?)

User Experience
The new UI works by adding the search/replace definitions to the table widget. This means a user may enter (and test) a search/replace definition in the RegexEdit/Textbox widgets and forget to press Add.
There are two ways I can think to solve this:

1. Leave it as it is, the user will learn.
2. Pop a dialog asking the user whether to add the definition in the RegexEdit/Textbox widgets if it not already there when the leaving the Search/Replace window.

Anything Else
Anything else I need to do to make this change merge worthy?
idea00 is offline   Reply With Quote
Old 04-10-2012, 10:46 PM   #2
kovidgoyal
creator of calibre
kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.
 
kovidgoyal's Avatar
 
Posts: 26,131
Karma: 5381911
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
Backwards compatibility:

You cannot remove the sr?_replace options. calibre saves previously used conversion settings. Your change would render the saved settings useless. Instead leave them be and add another option. Have the GUI use the new option for new conversions, pre-populating it with the values from the sr? options, if they exist. In the case where values exist for both, ignore the old ones.

Internationalization

You only need to mark strings with _() in .py files. .ui files are handled automatically. That's all you need to do.

UI

Why not use both the explicitly added expressions and the values in the textboxes, if they exist.

CLI

How are you serializing multiple expressions on a single command line option?
kovidgoyal is offline   Reply With Quote
Old 04-11-2012, 12:16 AM   #3
idea00
Junior Member
idea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enough
 
Posts: 7
Karma: 684
Join Date: Apr 2012
Device: android
Quote:
Originally Posted by kovidgoyal View Post
Backwards compatibility:

You cannot remove the sr?_replace options. calibre saves previously used conversion settings. Your change would render the saved settings useless. Instead leave them be and add another option. Have the GUI use the new option for new conversions, pre-populating it with the values from the sr? options, if they exist. In the case where values exist for both, ignore the old ones.
I need a bit of help with this as I do not entirely understand how options are handled.
As far as I've gathered the options in the UI are somehow connected to the OptionRecomendations in plumber.py using the functions get_value and set_value in the class Widget (gui2\convert\__init__.py)
I added support for handling QTableWidget widgets to this class and named my table opt_search_replace so it matches the option name (search_replace)

In order to support sr?_search/sr?_replace options I'd either need dummy widgets or would have to use a different mechanism than the one used in the original search_and_replace.py. Can you point me in the right direction for this?

Quote:
Originally Posted by kovidgoyal View Post
UI

Why not use both the explicitly added expressions and the values in the textboxes, if they exist.
That's the less intuitive option, it means if a user just messes around with search/replace and decides not to use it, he now needs to delete the regex from the RegexEdit widget not just remove everything from the table.


Quote:
Originally Posted by kovidgoyal View Post
CLI

How are you serializing multiple expressions on a single command line option?
Forgot to ask about this.
I use json.
Widget.get_value gets the text values of the table into a list (of lists) and json encodes it.Widget.set_value reverses the process.
The save and load load functions use the same functions to serialize/deserialize the search/replace definitions into a file.
A bit ugly but less computationally intensive than dynamically generating search and replace options. The original approach of using two options for what is in essence a single function was uglier still as one could potentially disable just a replace option without disabling the matching search option.
The best approach would be for options to support lists natively not just numbers and strings, but I have not yet deciphered how to do it or know the implications of such a change.
idea00 is offline   Reply With Quote
Old 04-11-2012, 12:33 AM   #4
kovidgoyal
creator of calibre
kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.
 
kovidgoyal's Avatar
 
Posts: 26,131
Karma: 5381911
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
Don't change the Widget class. Instead implemnt get_value_handler and set_value_handler in the S&R class to special case the handling of this option. You can be as flexible as you want in those methods. A grep in that directory will show you examples of using those methods.

On the GUI:

There are two less than optimal scenarios:

1) The user enters something but does not click add. I which case his expression is ignored
2) The user enters something but does not click clear, in which case the expression is applied even though he did not intend it to be applied.

To me, scenario (1) is preferable, as it makes the feature easier to use for newbies. You can always add a popup that pops up only if there is text in those boxes and asks the user if he wants to apply that text or not. Do these by subclassing commit() in the S&R widget.

On the CLI:

You wont be able to find a serialization scheme that is both robust and easy to enter given the quoting restrictions of most shells. Instead, I suggest using the path to a file for the CLI.If specified, load the expressions from that file. To make it human usable have the file format be something simple like
Code:
single line regex1
replacement expression1

single line regex2
replacement expression2
This also allows for an easy analogue of the save load functionality from the CLI. You will also be able to use this option in the GUI, by writing your table into a temp file and passing it using this option.

Last edited by kovidgoyal; 04-11-2012 at 12:36 AM.
kovidgoyal is offline   Reply With Quote
Old 04-11-2012, 09:42 AM   #5
idea00
Junior Member
idea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enough
 
Posts: 7
Karma: 684
Join Date: Apr 2012
Device: android
Quote:
Originally Posted by kovidgoyal View Post
Don't change the Widget class. Instead implemnt get_value_handler and set_value_handler in the S&R class to special case the handling of this option. You can be as flexible as you want in those methods. A grep in that directory will show you examples of using those methods.
The methods are barely used by the classes in that directory, but looking at Widget again I see how they work. The methods still get passed a property of the main class (the g parameter) not the name of the desired option.
I'll still need the opt_sr* properties but I can get it to work by saving the name of the option in the property.

I'll revert the changes to Widget and implement: get_value_handler, set_value_handler and connect_gui_obj_handler.

Quote:
Originally Posted by kovidgoyal View Post
On the GUI:

There are two less than optimal scenarios:

1) The user enters something but does not click add. I which case his expression is ignored
2) The user enters something but does not click clear, in which case the expression is applied even though he did not intend it to be applied.

To me, scenario (1) is preferable, as it makes the feature easier to use for newbies. You can always add a popup that pops up only if there is text in those boxes and asks the user if he wants to apply that text or not. Do these by subclassing commit() in the S&R widget.
I was thinking on the lines of less damage. Scenario 1 may cause the user to loose text from books should the original be deleted after the (erroneous) conversion, scenario 2 will merely not do the job but no data will be lost.
In any case I'll go with the popup option which is the best and also clear the fields for search/replace in the RegexEdit and Textbox when the user adds or changes a S/R definition in the table.

Quote:
Originally Posted by kovidgoyal View Post
On the CLI:

...
Sorry just now the acronym CLI (Command Line Interface) clicked. I thought you where referring to something else in your previous post and hence the answer.
I'm going to restore backward compatibility so the CLI for all the original sr* options should be restored and use a single human parsable serialization scheme for both the UI saved files and the CLI.
idea00 is offline   Reply With Quote
Old 04-13-2012, 12:31 AM   #6
idea00
Junior Member
idea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enough
 
Posts: 7
Karma: 684
Join Date: Apr 2012
Device: android
Ok I finished with all the open issues and sent you a merge proposal on lp.

Backward Compatibility
Fully backwards compatible any sr?_search / sr?_replace option saved in the conversion definitions are automatically converted to entries in the search_replace option and they can be used in ebook-convert as usual.


User Interface
Reverted Widget and implemented: get_value_handler, set_value_handler and connect_gui_obj_handler.
I still needed to change Widget to include setup_help_handler (as a counterpart to setup_help) in order not to have to use dummy widget for the original sr* options.

CLI

Quote:
Originally Posted by kovidgoyal View Post
You wont be able to find a serialization scheme that is both robust and easy to enter given the quoting restrictions of most shells. Instead, I suggest using the path to a file for the CLI.If specified, load the expressions from that file. To make it human usable have the file format be something simple like
Code:
single line regex1
replacement expression1

single line regex2
replacement expression2
This also allows for an easy analogue of the save load functionality from the CLI. You will also be able to use this option in the GUI, by writing your table into a temp file and passing it using this option.
I can't just use a temporary file for the option as the options are saved for each ebook and for bulk conversion when using the GUI so I'm using a dual approach: json or a file can be accepted.
The GUI will always return json so it can be saved along with the other options and used during conversion without requiring an external file.

I hope you have time soon to review my proposal and that it can make it to the trunk.
idea00 is offline   Reply With Quote
Old 04-13-2012, 11:26 AM   #7
kovidgoyal
creator of calibre
kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.
 
kovidgoyal's Avatar
 
Posts: 26,131
Karma: 5381911
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
Merged, changing to remove unneccessary file/json duality and also some miscellaneous cleanups/fixes. I still have to test backward compatibility, but otherwise it looks good.
kovidgoyal is offline   Reply With Quote
Old 04-15-2012, 09:26 PM   #8
idea00
Junior Member
idea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enough
 
Posts: 7
Karma: 684
Join Date: Apr 2012
Device: android
Thanks.
I saw what you did, nice.
If there are any bugs I need to take care of email me.
idea00 is offline   Reply With Quote
Old 04-21-2012, 03:25 AM   #9
_jago
Junior Member
_jago began at the beginning.
 
Posts: 2
Karma: 10
Join Date: Apr 2012
Device: none
I use the S&R funktion frequently and appreciate the new version very much. Thank you for that I love it.
But if I am not totally wrong, the execution order of the regex seems not predictable and that's bad for some special cases.
I would have expected that the execution is in order from 1 to N.
Can this be changed? And if so it would be nice to have the easy posibility to change the position of every regex in the stack (e.g. up/down arrows).
_jago is offline   Reply With Quote
Old 04-21-2012, 03:54 AM   #10
kovidgoyal
creator of calibre
kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.kovidgoyal ought to be getting tired of karma fortunes by now.
 
kovidgoyal's Avatar
 
Posts: 26,131
Karma: 5381911
Join Date: Oct 2006
Location: Mumbai, India
Device: Various
IIRC, the order is preserved. Try emailling idea00 to see if he is willing to implement the order movement buttons. You will find his email address in the calibre source code file gui2/convert/search_and_replace.py
kovidgoyal is offline   Reply With Quote
Old 04-29-2012, 08:11 AM   #11
idea00
Junior Member
idea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enoughidea00 will become famous soon enough
 
Posts: 7
Karma: 684
Join Date: Apr 2012
Device: android
Merge request sent.

I've added reorder arrows to the UI and fixed a few minor UI logic bugs.

The order in which the S/R expressions are applied in the current version is reverse (hence _jago stating the order does not seem predictable.)
The reason is the function "do_search_replace" in preprocess.py inserts s/r rule at the start of the rules list.
I assume there is a reason for this so I fixed this by using a revered() iterator, if not appending to the rules would also solve this issue.
idea00 is offline   Reply With Quote
Old 05-03-2012, 04:09 AM   #12
_jago
Junior Member
_jago began at the beginning.
 
Posts: 2
Karma: 10
Join Date: Apr 2012
Device: none
Wow! Didn't expect such a fairly quick response to my suggestions. Thank you for your effort to make this nice funktion much more usefull for me (hopefully not only for me).
Quote:
The order in which the S/R expressions are applied in the current version is reverse
Ok, that explains my confusion with the execution order.

But anyway as soon as possible I will test the new functionallity.
_jago is offline   Reply With Quote
Reply

Tags
calibre, developement

Thread Tools Search this Thread
Search this Thread:

Advanced Search

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Search and Replace SOThunder Conversion 2 04-29-2012 08:29 AM
Search and replace TdeV Sigil 1 10-30-2011 04:45 PM
search and replace - drops blanks in replace ? cybmole Conversion 10 03-13-2011 03:07 AM
Search and replace in 0.2.0 paulpeer Sigil 7 03-13-2010 11:59 AM
Why no search and replace? charleski Sigil 10 11-24-2009 04:13 PM


All times are GMT -4. The time now is 06:49 PM.


MobileRead.com is a privately owned, operated and funded community.