← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~m2j/openlp/formattingtags into lp:openlp

 

> Had 5 mins to have a look and I must say it is better but still cannot
> understand why it needed a re-write.
Well, the initial reason was to add some validation. Especially to enforce correct opening and closing tags. Without this it is not clear, how to handle custom tags in my new lyrics editor draft.
I had to change some things and looked for some more improvements, as the old editor is very slow in use (in the way, that you need much actions to perform a change).
Finally, I had some problem with the current architecture, which does every changes directly in FormattingTags(). This did not work together with some of my changes, and with the paradigma to separate the UI and data.
So, there where nothing significant left, which I could reuse.
This just as a explanation, why I did a rewrite. I might be wrong in some points.

> 1) Restore Defaults should set the list back to the default list.  It does
> not.
Oh, you're right. I wanted to place some "revert" button. I picked some wrong naming and didn't noticed it.

> 2) Hit new and it creates a new line.  Good.  Add a tag and another new line
> appears.  Bug
I see. It does not add a new line to focus on. I will fix it.

> 3) You cannot delete the final blank line it is always present cos you need a
> tag to enable delete and this auto creates a new line.
Yes and no. Yes, you cannot delete the final blank line and no, it's not because you need a tag, but because I wrote a query, which disables the delete function for the final line. Seems, as it works exactly as intended.

The final empty line is there to provide a quick and simple method to add new tags. Without such a line, you would have to press the "new" button each time, or use some hidden "magic". Do you see any problem or confusion of users due to the blank line? I think, that the role of a blank line at the end of a list is obvious.

The only thing, you cannot do is to enter a new tag and press delete without finishing the edit. But what's the sense of that?

> Playing with random inputs gave me
> Traceback (most recent call last):
> File "/home/tim/Projects/openlp/formattingtags/openlp/core/ui/formattingtagfor
> m.py", line 153, in on_current_cell_changed
> item.setText(data.get(u'tag'))
> TypeError: QTableWidgetItem.setText(QString): argument 1 has unexpected type
> 'NoneType'

Thanks. Seems, that I've missed this case. I'll fix it.
-- 
https://code.launchpad.net/~m2j/openlp/formattingtags/+merge/140652
Your team OpenLP Core is subscribed to branch lp:openlp.


References