Two problems with splitting files

[BeckyEbook]

First case

1. Open any file, place the cursor in front of any open or close tag, and try to split the file (Split At Cursor).

Message: File cannot be split at this position.

Why? After all, it is enough to move the cursor one field to the left and then the split will work.


Second case

1. Open any file, but change the extension from xhtml to xml (I see files like that in the wild quite often).
2. Place the cursor anywhere (apart from the one I described in the first case of course) and try to split the file (Split At Cursor).

Message: Cannot split since it may not be an HTML file.

Despite the message, the split will be performed ... but the split part of the file will be lost (we lose a piece of code BEFORE split position, and it remains AFTER split)

[KevinH]

The cursor is actually on the first <, not "before it.
so if CV shows something like |<tag> , then the cursor is in the tag. If you had a block cursor it would highlight the < but CV uses an insertion point cursor.

The long held Sigil rule is you can not split when in a tag. If you think this is too tight, the I would be happy to consider a patch or PR that handles just that one case.


Note, a file with an xml file extension can be of any type, it could be an opf, an ncx, an xhtml, a generic xml file, etc. There is no way to know the tag meanings: a body tag may not be what you think it means, there may be no html root tag, there is no way to tell which are void tags and which are not, which are block tags and which are not, where it might even be safe to split, and how splitting will impact a generic xml file.

And according to the epub3 spec, file extension must be xhtml if the file is an xhtml file.

The first step when seeing lots of xml files that are really xhtml files (or html files that are really xhtml files) is to use Sigil's ability to bulk rename a set of files by just changing the extension as they do not meet the current epub spec left as .xml files.

Again, I would consider a patch or PR to handle this differently but Sigil has long operated this way, and forcing people to change file extensions to more correct ones that meet the latest spec makes good sense to me. Please note, that old epub2 spec that allowed pure xml "islands" has long been deprecated mainly since no reader bothered to support it and just treated all .xml files as .xhtml files. This changed in epub3.

Hope this explains why you are seeing what you are seeing.

Originally Posted by BeckyEbook

First case

1. Open any file, place the cursor in front of any open or close tag, and try to split the file (Split At Cursor).

Message: File cannot be split at this position.

Why? After all, it is enough to move the cursor one field to the left and then the split will work.


Second case

1. Open any file, but change the extension from xhtml to xml (I see files like that in the wild quite often).
2. Place the cursor anywhere (apart from the one I described in the first case of course) and try to split the file (Split At Cursor).

Message: Cannot split since it may not be an HTML file.

Despite the message, the split will be performed ... but the split part of the file will be lost (we lose a piece of code BEFORE split position, and it remains AFTER split)

[exaltedwombat]

I can reproduce the second issue, but not the first. Sigil 1.4.3 Windows 10.

[KevinH]

The point I was trying to make is both are expected behaviour. So a patch and rationale to change this would be needed for either.

[DiapDealer]

If a user gets the message "Cannot split since it may not be an HTML file" then why is the split still being performed (with the subsequent loss of data)? Shouldn't the split attempt be aborted under those conditions if possible?

[KevinH]

Originally Posted by DiapDealer

If a user gets the message "Cannot split since it may not be an HTML file" then why is the split still being performed (with the subsequent loss of data)? Shouldn't the split attempt be aborted under those conditions if possible?

Probably. That code is either in CodeViewEditor.cpp itself or in MainWindow.

That I can look at.

[KevinH]

Well the first "issue" can be found in CodeViewEditor.cpp here if anyone wants to play with that, they could simply check for a |<tag> here and allow that even if "considered in tag".

QString CodeViewEditor::SplitSection()
{
    QString text = toPlainText();
    int split_position = textCursor().position();

    // Abort splitting the section if user is within a tag - MainWindow will display a status message                           
    if (IsPositionInTag(split_position, text)) {
        return QString();
    }

I do not see that error message "Cannot split since it may not be an HTML file" anywhere in CodeViewEditor though. But in MainWindow.cpp that error message comes here:

void MainWindow::CreateSectionBreakOldTab(QString content, HTMLResource *originating_resource)
{
    if (content.isEmpty()) {
        QMessageBox::warning(this, tr("Sigil"), tr("File cannot be split at this position."));
        return;
    }

    // XXX: This should be using the mime type not the extension.                                                               
    if (!TEXT_EXTENSIONS.contains(QFileInfo(originating_resource->Filename()).suffix().toLower())) {
        QMessageBox::warning(this, tr("Sigil"), tr("Cannot split since it may not be an HTML file."));
	return;
    }

But by this point the "damage" has been done and the original file is now doomed.

It seems SplitAtCursor is handled inside CodeView where it takes the bottom half of the file and uses the current file name for it and then the piece above under a new name. All of this was designed before SplitAtSplitMarkers was done.

Hmm....

[KevinH]

Okay, this needs to be handled in FlowTab.cpp SplitSection() before the OldTabRequest is fired as that is obviously too late.

void FlowTab::SplitSection()
{
    if (!IsDataWellFormed()) {
        return;
    }

    QWidget *mainWindow_w = Utility::GetMainWindow();
    MainWindow *mainWindow = qobject_cast<MainWindow *>(mainWindow_w);
    if (!mainWindow) {
        Utility::DisplayStdErrorDialog("Could not determine main window.");
        return;
    }
    HTMLResource * nav_resource = mainWindow->GetCurrentBook()->GetConstOPF()->GetNavResource();
    if (nav_resource && (nav_resource == m_HTMLResource)) {
        Utility::DisplayStdErrorDialog("The Nav file can not be split");
        return;
    }

    // Handle warning the user about undefined url fragments.                                                                   
    if (!mainWindow->ProceedWithUndefinedUrlFragments()) {
        return;
    }

    if (m_wCodeView) {
        emit OldTabRequest(m_wCodeView->SplitSection(), m_HTMLResource);
    }
}

So from this, I think we can simply remove the "Cannot be split" from that part of MainWindow.cpp as it is too late by that point to not hurt something.

We can then "*assume*" that a .xml file that actually gets successfully loaded into a FlowTab itself must be xhtml (otherwise it would be loaded in a XMLTab) and maybe even remove that test in MainWidow completely since it is already too late.

I will try doing that.

[KevinH]

@BeckyEbook,
I just pushed a change to master for your second "issue". When you get a free moment, please give it a try and let us know.

At least half of the file should not be missing anymore but there is no guarantee that a generic xml file will be split properly either. Only xhtml is this designed to work for.

[BeckyEbook]

I can confirm that it is better now – the second case does not delete part of the file
Thank you.

[DiapDealer]

Thanks, Kevin.

[KevinH]

@BeckyEbook,

I looked at the SplitSection code and the actual split position is *not* included in the spilt (it grabs everything before that char) so I can check for the text[pos] == '<' and special case that in SplitSection so |<tag> would then work as a split point.
I will push that to master so you can try it and let us know if that works for you.

Kevin

[KevinH]

@BeckyEbook
Okay just pushed that change. It appears to work to me.

I had it look like the following:

<div>top</div>|<div>bottom</div>

with the cursor being represented by the '|' char and hit Split At Cursor.
I got back what I expected.

If this actually worked in Sigil 1.4.3 and earlier it was by accident.
Now it will make all earlier behaviour official.

Thanks for your bug reports!

[BeckyEbook]

I had exactly this idea for solving the first problem, but the idea and the implementation in code are two different things.

Splitting works great now! Thank you, again.
I will try to do more tests in the near future.

BTW, your sample code from the previous post causes an unexpected side effect.
Paste this fragment and call Mend & Prettify several times:

<div>top</div>|<div>bottom</div>

I know this is an incredibly rare situation, I report it as a curiosity.

[DNSB]

Originally Posted by BeckyEbook

BTW, your sample code from the previous post causes an unexpected side effect.
Paste this fragment and call Mend & Prettify several times:

<div>top</div>|<div>bottom</div>

I know this is an incredibly rare situation, I report it as a curiosity.

Cute. So no naked pipe symbols allowed.