Header/footer fixes (#28875 and #25245)

[Thomas-Ganter]

Resolves: #28875
Resolves: #25245

Headers and Footers were wrongly formatted because the Page Number and Copyright overrides were used as the default styles instead of the Header and Footer presets. These two changes fixes that.

• I signed the CLA
• The title of the PR describes the problem it addresses
• Each commit's message describes its purpose and effects, and references the issue it resolves
• If changes are extensive, there is a sequence of easily reviewable commits
• The code in the PR follows the coding rules
• There are no unnecessary changes
• The code compiles and runs on my machine, preferably after each commit individually
• I created a unit test or vtest to verify the changes I made (if applicable)

[mathesoncalum]

Thank you very much for this @Thomas-Ganter! This part of the code is incredibly fragile but I think your changes will work. There are a couple of cleanup points I'd like to address as well because it seems the current implementation of style was rendered somewhat redundant after #24442...

The following lines can be deleted because replaceTextMacros/formatForMacro should (hopefully) handle the unique formatting of copyright and page numbers:

MuseScore/src/engraving/dom/page.cpp

Lines 99 to 109 in 6d16859

	//! NOTE: Keep in sync with replaceTextMacros
	std::wregex copyrightSearch(LR"(\$[cC])");
	std::wregex pageNumberSearch(LR"(\$[pPnN])");
	bool containsCopyright = s.contains(copyrightSearch);
	bool containsPageNumber = s.contains(pageNumberSearch);
	// Slight hack - we'll use copyright/page number styling if the string contains copyright or page number
	// macros (hack because any non-copyright text in the same block will also adopt these style values)
	TextStyleType style = containsCopyright ? TextStyleType::COPYRIGHT
	: (containsPageNumber ? TextStyleType::PAGE_NUMBER
	: (isHeader ? TextStyleType::HEADER : TextStyleType::FOOTER));

Then, at the second formatting pass:

MuseScore/src/engraving/dom/page.cpp

Lines 166 to 169 in fa0f99c

	// second formatting pass - replace macros and apply their unique formatting (if any)
	std::vector<TextBlock> newBlocks;
	for (const TextBlock& oldBlock : text->ldata()->blocks) {
	Text* dummyText = Factory::createText(score()->dummy(), style);

Declare style like this:

    // second formatting pass - replace macros and apply their unique formatting (if any)
    const TextStyleType style = isHeader ? TextStyleType::HEADER : TextStyleType::FOOTER;
    std::vector<TextBlock> newBlocks;
    for (const TextBlock& oldBlock : text->ldata()->blocks) {
        Text* dummyText = Factory::createText(score()->dummy(), style);

Hopefully that all works. Once we're done we'll need to test that copyright and page number formatting still works as expected, alongside custom XML formatting. If you have any questions let me know!

[Thomas-Ganter]

Hi @mathesoncalum , first time I even heard about custom XML formatting was reading it in the code. A brief manual search came back inconclusive.

Can you give me a pointer then I'll see that it won't break doing these changes.

[Thomas-Ganter]

... and yes, I figured the code being fragile and somewhat redundant, but I wanted to make the minimum change possible without a clearer understanding of things like custom XML formatting ...

Still, this formatting issue was so annoying to my sense of beautiful sheet music I had to do something. "How hard can it be, I was pretty good in C forty years ago ..." I thought. 😁

[mathesoncalum]

Can you give me a pointer...

Sure - from what I can tell it's not such a well known feature but it allows you to use XML formatting tags in the header and footer fields. In this example I put <font face="Arial"/><b><i>formatted text</i></b> in the middle field:

Playing around with it myself I didn't notice any regressions after applying your changes. There are some known bugs in this area though - see #25245 (something else on my todo list)...

[Thomas-Ganter]

Uah -- I immediately can think of 100+ things that can go wrong allowing arbitrary XML in text fields like this … beginning with what if I actually want xml-like tags in head/ footer fields?

But as I already have embarked on making some modifications here I'll have a look at that as well.

Only your <font face="Arial"/> example above makes me curious whether it's even correct, as you immediately close the tag it should not have any effect ...

[Thomas-Ganter]

Sorry, The Coding Style script does not run on my Mac ...

~/Projects/MuseScore^28875-Header-Footer-Styles ±
% git show --name-only '*.h' '*.cpp' | xargs tools/codestyle/uncrustify_run_file.sh                                                                                                         25-07-23 - 11:15:46
Option<NUM>: at tools/codestyle/uncrustify_musescore.cfg:209: Expected number , for 'indent_comma_paren'; got 'false'
Option<NUM>: at tools/codestyle/uncrustify_musescore.cfg:212: Expected number , for 'indent_bool_paren'; got 'false'
tools/codestyle/uncrustify_musescore.cfg:296: option 'sp_balance_nested_parens' never works; it has been replaced by 'sp_paren_paren'.

Option<UNUM>: at tools/codestyle/uncrustify_musescore.cfg:873: Expected unsigned number , for 'align_nl_cont'; got 'false'
tools/codestyle/uncrustify_musescore.cfg:950: option 'nl_func_var_def_blk' is deprecated; it has been replaced by 'nl_var_def_blk_end_func_top'.
You can also use 'nl_var_def_blk_end' for additional functionality.

Option<UNUM>: at tools/codestyle/uncrustify_musescore.cfg:1410: Expected unsigned number , for 'mod_full_brace_if_chain'; got 'false'
tools/codestyle/uncrustify_musescore.cfg:1560: option 'pp_space' is deprecated; it has been replaced by 'pp_space_after'.

Failed to load (commit)

Are there steps missing (or did I miss a step) in the "set up your environment" documentation?
I very much wanted to follow the coding style established by the existing code.

[cbjeukendrup]

@Thomas-Ganter the secret is that you specifically need Uncrustify version 0.74, which you will have to compile yourself.

If that still doesn't work, an easier way to fix the code style might be running PATH=${UNC_DIR}:$PATH tools/codestyle/uncrustify_run.sh, where ${UNC_DIR} represents the path to the folder containing the uncrustify 0.74 binary.

---

Regarding the <font /> tags: MuseScore has a somewhat unusual XML encoding of formatted text, for historical reasons. For fonts, it uses these self-closing tags, meaning that all text after such tag will use the specified font, until there is another <font/> tag.
And regarding the desirability of manual XML input in those fields: it's indeed questionable, but let's not question it now :). The real solution will be to replace these text fields with proper WYSIWYG text editors, but that's for another day.

[Thomas-Ganter]

There still were issues with text after a $p macro, in the wake fixing this and thus making headers great again™ I almost fixed #25245 , is it permissible to include this as well or is that considered Bad Taste (@mathesoncalum)?

(the remaining empty custom XML formatting tags issue depicted above I will also fix)

[mathesoncalum]

@Thomas-Ganter if you think you have a fix (or improvement) ready for #25245 then absolutely feel free to include it in this PR.

Quick aside - it looks like there was maybe some problem rebasing this PR. It now includes a bunch of extra commits.

[Thomas-Ganter]

I feel I now have #28875 and #25245 tackled. Along the way I encountered and fixed a reproducible crash caused by font size 0 being parsed from the <font size=" tag while typing if you (like me) type the closing tag prior to filling the content ...

[mathesoncalum]

Thanks @Thomas-Ganter! A few requested changes for you...

[mathesoncalum]

Almost there @Thomas-Ganter - left a couple of tiny tweaks for you and a tip about the unit tests.

Once you're done could you squash your commits (ideally into 2 commits - one for #28875 and one for #28875) and try to sign the CLA again?

Cheers!

[Thomas-Ganter]

With all the changes and the tests fixed, the original intent is still fulfilled: Header (and footer) formatting works again.

Squashing into separate commits is not easily possible, as the fixes are intertwined. Tried to separate them out as much as possible.

@mathesoncalum hope everything is hunky-dory now.

[mathesoncalum]

Hunky-dory indeed 😁

[Thomas-Ganter]

OK, I'm able to demonstrate that the deviation is due to the fix for #27519 :
This is the build compared with (right) and without (left) that fix:

Now ... adding Text into the footers gives:

I'll add a fix ontop of #27519 insofar that it will only include empty lines if the empty line is not the only line in header/footer.

[Thomas-Ganter]

Yesss ...

Now let's see about those VTests (that did not work on my mac according to the instructions provided ... and this was quicker.

[Thomas-Ganter]

@mathesoncalum fixing the fix fixed the tests

[mathesoncalum]

Nicely done @Thomas-Ganter - cheers!

@zacjansheski ready for re-testing

[zacjansheski]

Tested on MacOS 15, Windows 11, Ubuntu 22.04.3. Approved
#28875 FIXED
#25245 FIXED

[mike-spa]

@Thomas-Ganter @mathesoncalum this check turned out to be problematic, see #29906. The problem is that we use a variable called UNDEFINED_FONT_SIZE which is equal to -1 (i.e. intentionally invalid) but this check means that we write a valid pt size instead. If this was covering a crash, we need to find a proper solution for it.