Songs with mismatching formatting tags still throw an exception

I think the problem is, that the method '_get_missing_tags' only checks for 'not closed' tags, but not for 'not opened'.
I'm nearly a Phyton newbie, therefore I don't understand the complete code around this method and the following calls in 'song_to_xml', so I'm sorry that i cannot fix the problem for myself.

I wrote a function which checks the consistency of the tags in the verse.

The question is: What is the appropriate reaction of the program if the function returns there is a malplaced tag?
Should the whole import be canceled?

I think there is really two parts to this bug.

1. We should validate the formatting tags when a user enters a song manually:
    a) For 2.0 series think we should display a message box saying that the formatting tags are invalid.
    b) For trunk series, I was thinking it would be nice if we could highlight the actual issues. Either with a message box for example "x opening tag found, but no closing tag" or by actually changing the background colour of the tag to red or something.

2. For songs that are being imported or songs that are already in the database we should either remove the tag, or close it just before the next closing tag or split.

1a) So the users can't leave the editor before fixing the malformed tags? This would be the easiest solution, but not very user friendly. The user should have the possibility to save a song every time, even when it is malformed. Anyway, a message box should point out the user that there is an error an that the song could not be used in this state.

1b) This would be the advanced version of 1a, showing the wrong tags by coloring them. Nice.

2. I don't think so. In my opinion there mustn't be any changes on any song without the knowledge of the user.
You don't know, if there is a tag too much or too less or if this is a wrong written tag or ...
The user has to decide what to do with this malformed song.
Possible actions would be: Cancel import or show song in editor. Maybe ignoring the issue would be a possibility, too.

> 1a) So the users can't leave the editor before fixing the malformed
> tags? This would be the easiest solution, but not very user friendly.
> The user should have the possibility to save a song every time, even
> when it is malformed. Anyway, a message box should point out the user
> that there is an error an that the song could not be used in this state.

That would just be a stop gap soloutuon for 2.0. I think this is the most
as we can do seeming as we are not adding any new features to this release.
Its more user friendly than the current solution of showing the user a
criptic error message!

> 1b) This would be the advanced version of 1a, showing the wrong tags by
coloring them. Nice.

This would be for trunk (a.k.a 2.1) which we are adding features to.
Andreas' input on this would be valuable as he was having a look at
creating a WYSIWYG editor for the songs.

> 2. I don't think so. In my opinion there mustn't be any changes on any
song without the knowledge of the user.
> You don't know, if there is a tag too much or too less or if this is a
wrong written tag or ...

In which case we should just ignore it. The last thing I would want is to
have to find and show a song during spontaneous worship, and then for
OpenLP to force me to edit it before showing it.

> The user has to decide what to do with this malformed song.
> Possible actions would be: Cancel import or show song in editor. Maybe
ignoring the issue would be a possibility, too.

Unfortunately with a lot of the song importers we just have to make
assumptions about what we're importing. I don't think there is a single
importer which keeps formatting data (with the exception of possibly
OpenLyrics). We could log which files we had to drop any formatting from in
the import wizard.

1a) I see. So the best solution for 2.0 series would be a message box when the user leaves the song editor, without let him out before he has fixed the problem. I think with my quick'n'dirty function this should be no too hard. I'll try that at the weekend.

2) I do not know anything about the importers, so I guess you are right ;) A logging about the issues while the import should be a must have. The question is: Should this be implemented in 2.0? As you said, it's not very nice if you import a song quickly for a spontaneus worship and when the song should be displayed, OpenLP fires an exception...

1a) is done for 2.0 series. See linked branch.
I'm not sure if I should propose for merging, because it's assigned to V2.0.3. Maybe I should wait until 2.0.2 is released?

Additionally we should probably try and catch this error and issue a friendly warning to the user.