Oddity in e²->e³ conversion

[DiapDealer]

Originally Posted by KevinH

So at best Sigil can simply set the modified flag to true on any initial load of any epub because the OPF whitespace will most probably have been changed at the very least.

I wonder if that's a good idea though. Won't it result in a lot of questions about why they got prompted to save the epub when they close it when they made no edits? Or have I misunderstood?

[philja]

Originally Posted by DiapDealer

I wonder if that's a good idea though. Won't it result in a lot of questions about why they got prompted to save the epub when they close it when they made no edits? Or have I misunderstood?

A little pop-up with the prompt to save, just to give a small explanation?

[KevinH]

That is the key point. Is a whitespace only change of the opf relevant to saying the book was modified?

It does change validator line numbers but as I said, a validator plugin should always use Sigil's internal files.

[DiapDealer]

Originally Posted by KevinH

That is the key point. Is a whitespace only change of the opf relevant to saying the book was modified?

It isn't in my opinion. Especially since it hasn't been relevant for years.

[KevinH]

So if whitespace changes of the opf are fine, I could set modified true on initial load only when not well formed initially (and add a LoadWarning) and was repaired (as its repair is more than a whitespace only change).

That is a middle ground that would not hurt and is doable.

How does that sound?

[DiapDealer]

I'm fine with that. I like the idea of notifying that a malformed opf has been repaired upon open regardless.

[KevinH]

Then that's the plan.

[Kayadelenium]

A note on what happens on my side, in case it is useful context rather than anything needing action.

My Sigil plugin builds what it validates from bk.copy_book_contents_to(workdir) and zips that, mimetype first and stored. It substitutes nothing afterwards. That last part was a change in 0.1.1 on 3 September, after a book where the OPF gave me one line number and the cursor landed on another.

What that leaves me depending on is one thing. Each finding's line and column is turned into an absolute character offset so the cursor can be placed on it, and that is only right when the copy I was handed matches what Code View is showing.

So the part I would like to understand, whenever it suits you:

1. With the change you have in mind, would a repaired-on-load OPF reach a plugin as the repaired text — that is, is the flag set before the flush that runs ahead of a plugin?

2. And would the same hold for XHTML mended on open? I have not measured this and am not suggesting anything is wrong. It is the same question one file type over, and my XHTML findings carry offsets computed the same way.

Thank you for the LoadWarning idea in particular. In #22 I asked whether a plugin could tell that Sigil had repaired something during load, and could not answer it myself — that answers it.

If testing would save you any time, I can drive launcher.py headless, so running a build against the shapes in this thread and reporting what a plugin receives is cheap here. Happy to do that, or to make a minimal book for anything you want isolated — and equally happy to leave you to it.

[KevinH]

You are confusing two things. If the plugin properly handles things, the plugin only ever sees what is stored inside Sigil's files by the PluginRunner's SaveAllToDisk which is run when any plugin is launched but the files encoding will always be utf-8 based.

If I read that file as a python string, it will be utf-32 encoded.

But CodeView just like every Qt based editor, the text file has had all carriage returns removed upon load and converted to utf-16 le (encoded) to create QString/QChars. And offsets are in QChars within the editor. These are not the same as utf-8 columns and offsets but line numbers should remain the same.

Which is great if your OS is native utf-16 string based, and a real pain in the ass if assuming utf-8 strings (most languages) or utf-32 (python). So you will need to convert your line, column, offsets from their generated encoding values to utf-16 based equivalents and account for any crs that are stripped out on load and only added on file saves on Windows (not Linux or MacOS).

I wrote a routine to do that for epubcheck for java generated lines, columns and offsets. I have no idea what Rust's string encoding uses and how your program generates line, column and offset errors in files so you may need to tweak that routine to work for you.

In general the best way to handle those conversions depends on original encoding, target encoding, and line end rules. But splitting each file into lines is typically the first step, and keeping track of the offset of each newline in both encodings and using character encoding conversion to get column info, can greatly speed up converting line, column, offset from one system to another.

What is reported by the validation routine must be consistent with Qt's utf-16 le, with no crs to work for an exact position versus the line which can be directly mapped while column info can be easily found by a substring encoding conversion since it must start fresh on a newline.

Hope that makes sense if I understood what you were asking. The key point is always use the files presented via the plugin's copy_ebook_contents interface and they should "match" what Sigil loads into CodeView aside from encoding and line end differences that are inherent with Qt's utf-16 le QString and QChar handling.

[KevinH]

No need to launch anything headless to test anything or develop a plugin. Just temp edit your plugin.py to always return -1 and all stderr and stdout will be saved including any python backtraces. So a real fast direct edit of plugin.py code versus results cycle for fast debugging is easily possible with no changes ever made to actual eoub you are testing with.

[KevinH]

And to clarify, all changes including auto mended charges are always flushed to Sigil's internal disk workspace first when any plugin is run. The problem came from the original epubveri plugin using the modification flag as an expedient to know when to try using the original epub file instead of what is inside Sigil on disk. Validators should *always* validate what is inside Sigil's workspace. That is what will be saved if and when a save is done by the user.

That original epub path is basically useless unless the user just did a save and meaningless if a user does a save-as. It was added to the plugin interface to allow edit plugins to create diffs, or overwrite the original input file, before we migrated to git Checkpoints. It was never meant to be used in that way in validators. Doitsu's epubcheck plugin handles all if these things well.

[KevinH]

This change to detect malformed OPFs on initial load and add warning (and set modified) has now been pushed to Sigil master.

It will appear in our next release.

[Kayadelenium]

Originally Posted by KevinH

So you will need to convert your line, column, offsets from their generated encoding values to utf-16 based equivalents and account for any crs that are stripped out on load

That was worth more than you probably expected when you wrote it. It was three separate things on my side, and two of them were live on real books.

Line breaks. Python's str.splitlines() also breaks on U+2028, U+2029, U+0085, form feed and vertical tab. epubveri counts newlines, so one of those characters in a file gave my table a line the validator had not counted, and every finding after it pointed a whole line away. 4 books of the 474 I test against carry one, 35 occurrences between them.

Astral characters. Exactly your QChar point. One emoji or mathematical letter is one Python character and two QChars, so every finding after one landed a position early. 1 book of 474, with 221 of them in it.

Carriage returns — already right, and right by accident. This is the one I would pass on to anyone else writing a plugin in Python: open() in text mode translates CRLF to LF by default, so the CRs are gone before any of my arithmetic sees them, and my offsets happened to match Code View without anybody deciding they should. Adding newline="" to that one call — which looks like a tidy-up — moves every finding past line 1 on 61% of real books, on Windows only, which is not where I develop. It has a test now rather than a default.

What the two real ones cost, measured rather than reasoned about: across the five books on my shelf that contain any of these characters, 40 findings move, the worst by 567 characters. All of it is invisible on macOS with ASCII content, so each fix was checked by breaking it again and watching the test fail.

Originally Posted by KevinH

Validators should *always* validate what is inside Sigil's workspace.

Agreed, and to be exact about my own history there: this plugin has built what it validates from copy_book_contents_to since its first version, and has never read the original .epub path or the modified flag. What it did do in 0.1.0 was overwrite the copied OPF with get_opf(), which is the same mistake one level down — validating something Code View was not showing. That went in 0.1.1 on 3 September, for the reason you give here: the file in the workspace is the thing.

Also took your advice in #40 — returning -1 to get stdout, stderr and backtraces out of a plugin is far quicker than what I was doing, and it needs no separate harness. Thank you.

And thank you for the answer to #38, and for the LoadWarning going into master. A plugin being able to tell that Sigil repaired something on load closes a question I could not answer from my side at all.

Shipped as 0.3.3.

[DNSB]

For anyone running Windows and who is interested in testing, you can download the installer from: Sigil-2.8.5-Windows-x64-Setup.exe.

The SHA-256 for the file is: 629EDEAD055E45C8B3169F7B84C39FA3B5D47745B7B53BB0D8 C84DC67AA5FB0B