Improves menu separator parsing with validity/visibility node parsing

[GasDauMin]

A change has been made, can you review whether it is worthy of being included in the next version?

I haven't had much time to create unit tests for this change, but I hope I haven't made any mistakes. I did the testing under realistic conditions.

[end2endzone]

No worries for the unit test. There is no "visual test" to create. I will only add a new one that test the parsing of a separator with validity or visibility elements.

Regarding your commit, I see no issue. Everything seems to be well managed.

I did the testing under realistic conditions.

I would assume that you tried your modified version in File Explorer. I see you parsed <visibility> and <validity> elements for separators. The menu rendering code was already quickly skipping over "invisible" menus (including now separators).

However, the rendering code would probably process a "disabled separator". Have you tried that? Could you show a screenshot of the following ?

    <menu name="issue 142">
      <menu name="test for disabled separators" />

      <!-- this a separator for baseline compare -->
      <menu separator="true" />

      <menu separator="true">
        <!-- this should be invisible -->
        <visilibty istrue="false" />
      </menu>
      <menu separator="true">
        <!-- this should be disabled -->
        <validity istrue="false" />
      </menu>
      <menu name="this is a separator with a name" separator="true">
        <visilibty istrue="true" />
        <validity istrue="true" />
      </menu>

    </menu>

[GasDauMin]

Very strange, the setting is not working, that means I haven't really assessed everything.

In other side for my cases, it works perfectly.

[GasDauMin]

I am afraid I will not have time to do a more proper analysis during the holidays as I will be away.

[GasDauMin]

No worries for the unit test. There is no "visual test" to create. I will only add a new one that test the parsing of a separator with validity or visibility elements.

Regarding your commit, I see no issue. Everything seems to be well managed.

I did the testing under realistic conditions.

I would assume that you tried your modified version in File Explorer. I see you parsed <visibility> and <validity> elements for separators. The menu rendering code was already quickly skipping over "invisible" menus (including now separators).

However, the rendering code would probably process a "disabled separator". Have you tried that? Could you show a screenshot of the following ?

    <menu name="issue 142">
      <menu name="test for disabled separators" />

      <!-- this a separator for baseline compare -->
      <menu separator="true" />

      <menu separator="true">
        <!-- this should be invisible -->
        <visilibty istrue="false" />
      </menu>
      <menu separator="true">
        <!-- this should be disabled -->
        <validity istrue="false" />
      </menu>
      <menu name="this is a separator with a name" separator="true">
        <visilibty istrue="true" />
        <validity istrue="true" />
      </menu>

    </menu>

I found a problem, the word "visibility" is spelled incorrectly in the above example. Validity rule adapts, but I don't really understand the term "disabled separator" in this context. After correcting the typo I see 3 separators as I understand they should be. To summarise, my tests passed :)

[end2endzone]

I am sorry for the typo. I must be more careful and constantly look out for these. I hope you did not waste too much time until you find that.

but I don't really understand the term "disabled separator" in this context.

According to the win32 API, you can define a separator and set the MFS_DISABLED or MFS_GRAYED flag. This would result in a "disabled separator" or a "grayed out separator". Such a separator is now possible with this update and the xml provided above. I was not sure how it would render or if it would even work. Thanks to your screenshot, I am now reassured.

I must also apologize for the big refactoring commit. I hope you did not get too many conflicts. I remember you talking about the chaotic commits and integration. I should have waited until this PR to be integrated before doing refactoring.

Regarding your code, I thank you for providing another contribution to the application. Much appreciated!

[end2endzone]

Reviewed. All code modifications approved.

[GasDauMin]

I am sorry for the typo. I must be more careful and constantly look out for these. I hope you did not waste too much time until you find that.

You don't need to apologise, it's your work that You're giving us all to use for free, and contributing to it is the least I can do to thank You for it.

I am most impressed by your programming discipline, the order that is everywhere, in the documentation, in the code, in version control. I still have a lot to learn from you and I am grateful for this opportunity.