OpenLP

Merge lp://qastaging/~suutari-olli/openlp/force-split into lp://qastaging/openlp

  1. force-split
  2. Merge into trunk
Proposed by Azaziah
Status: Superseded
Proposed branch: lp://qastaging/~suutari-olli/openlp/force-split
Merge into: lp://qastaging/openlp
Diff against target: 244 lines (+117/-10)
5 files modified
openlp/core/lib/renderer.py (+3/-3)
openlp/plugins/songs/forms/editsongform.py (+49/-0)
openlp/plugins/songs/forms/editversedialog.py (+6/-0)
openlp/plugins/songs/forms/editverseform.py (+43/-4)
tests/functional/openlp_plugins/songs/test_editverseform.py (+16/-3)
To merge this branch: bzr merge lp://qastaging/~suutari-olli/openlp/force-split
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing
Review via email: mp+294159@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2016-05-07.

This proposal has been superseded by a proposal from 2016-05-10.

Description of the change

- Added "Split" button for Song editor:
  This adds the currently selected Verses tag to
  the given position, thus creating a "Force split"
- Increased Optional Split limit from < 5 to < 51
- Increased test coverage

lp:~suutari-olli/openlp/force-split (revision 2663)
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-01-Pull/1538/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-02-Functional-Tests/1449/
[←[1;31mFAILURE←[1;m] https://ci.openlp.io/job/Branch-03-Interface-Tests/1387/
Stopping after failure

E:\bzr\openlp\force-split>pep8

E:\bzr\openlp\force-split>pep8

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote :

Cannot test as the UI is not visible.
There will be more comments when it works

review: Needs Fixing
Revision history for this message
Azaziah (suutari-olli) wrote :

Could you explain "Cannot test as the UI is not visible." ?

I replied to the other comments, thank you for the review.

Revision history for this message
Tim Bentley (trb143) wrote :

You have and Insert button which is not visible - simple.

Revision history for this message
Azaziah (suutari-olli) wrote :

> You have and Insert button which is not visible - simple.

Did you read my replies to code comments?

"What do you mean? The UI is the new "Split" button found in song editor,
in both "Edit all" and "Edit" modes."

This is visible for me, if it is not visible for you, then something has gone horribly wrong.

Revision history for this message
Tim Bentley (trb143) wrote :

Ok found it now as the button is only visible on Edit All.

All the button does is add a ---[Verse X]--- tag which is the same as the splt button does except that the split button has more options.

Why is this request needed because it would seem we have the required functionality already.

Revision history for this message
Tomas Groth (tomasgroth) wrote :

I think the idea is to force s split inside a verse - bot correct me if I'm wrong.
If that is the idea, then the button should also be available in single verse edit.

lp://qastaging/~suutari-olli/openlp/force-split updated
2664. By Azaziah

- Improved this on Edit single verse (This now detects if the verse type is changed)

Revision history for this message
Azaziah (suutari-olli) wrote :

> Ok found it now as the button is only visible on Edit All.
>
> All the button does is add a ---[Verse X]--- tag which is the same as the splt
> button does except that the split button has more options.
>
> Why is this request needed because it would seem we have the required
> functionality already.

----
> All the button does is add a ---[Verse X]--- tag which is the same as the splt
> button does except that the split button has more options.

Not sure if I understand this sentence, but:

With the "Split" button, the verse will be force splitted by using the verse tag.
This is easier than selecting the verse manually and then inserting it to the required position.
A lot of people are confusing the meaning of Optional split and try to perform force splits with it.
A lot of people have requested easier way to force split verses.

> Ok found it now as the button is only visible on Edit All.

Again: This is visible for me in: Edit, Edit all & Add,
are you sure this button is not visible for you as well?

Revision history for this message
Azaziah (suutari-olli) wrote :

> I think the idea is to force s split inside a verse - bot correct me if I'm
> wrong.
> If that is the idea, then the button should also be available in single verse
> edit.

This button is available in single verse edit, as well it is available in "Add" verse.
I believe TRB143 may has missed it or something very weird is happening.

lp://qastaging/~suutari-olli/openlp/force-split updated
2665. By Azaziah

Merged trunk (Noticed tests appear to be fixed)

2666. By Azaziah

- Fixed the issue whre verse current number is not updated properly on edit single verse mode.

2667. By Azaziah

Merged trunk?

2668. By Azaziah

Merged trunk?

2669. By Azaziah

Reverted the perfectly functioning Verse split technigue and replaced it with insert [--].
This still needs to be turned into force split in renderer.py

2670. By Azaziah

- This is broken / Going to revert to older solution after this.

2671. By Azaziah

- Reverted to the working solution.

Unmerged revisions

2671. By Azaziah

- Reverted to the working solution.

2670. By Azaziah

- This is broken / Going to revert to older solution after this.

2669. By Azaziah

Reverted the perfectly functioning Verse split technigue and replaced it with insert [--].
This still needs to be turned into force split in renderer.py

2668. By Azaziah

Merged trunk?

2667. By Azaziah

Merged trunk?

2666. By Azaziah

- Fixed the issue whre verse current number is not updated properly on edit single verse mode.

2665. By Azaziah

Merged trunk (Noticed tests appear to be fixed)

2664. By Azaziah

- Improved this on Edit single verse (This now detects if the verse type is changed)

2663. By Azaziah

Added a test for def on_insert_button_clicked(self):

2662. By Azaziah

Fixed a test

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.