gui-cs / TerminalGuiDesigner

Forms Designer for Terminal.Gui (aka gui.cs)
MIT License
423 stars 28 forks source link

Unit test conversion and TGD modernization round 2 #264

Closed dodexahedron closed 8 months ago

dodexahedron commented 8 months ago

Just continuing from #260

Feel free to merge whenever you like and I'll open another.

dodexahedron commented 8 months ago

A commit including the re-factor of the non-generic ViewFactory.Create method is going to be included in a merge to this branch, pretty soon. It condenses the whole thing down to a single 22-line return with a switch expression that delegates each type to the generic method, using the same type-matching rules and precedence as before, so that it is a non-breaking change.

There is no longer any duplicated code between the two methods.

I also put an obsolete attribute on it, just for ease of helping the migration to the generic version.

The generic version also has optional parameters for convenience in setting initial dimensions or default text, when relevant. The default values for all of them are identical to the old method's defaults.

I also moved the two instances of reflection that remained in the ViewFactory out into a separate ReflectionHelpers static class, since they're not really ViewFactory-specific, and potentially helpful for consolidation of other code, as the work to migrate to more ubiquitous generics goes on.

One instance of reflection also got eliminated entirely, as Terminal.Gui fixed the reason it was necessary in the first place (by putting a setter on a property we had to set via reflection, as it had no setter before), so that was a happy finding.

dodexahedron commented 8 months ago

Did a quick merge of the current state of the branch I'm working on, at the moment, primarily so you can finally see the fully refactored non-generic ViewFactory.Create method.

dodexahedron commented 8 months ago

Alrighty. For the tests that have been converted, coverage is as high as it can really get, for the most part, aside from some unreachable code that can be ignored for coverage purposes, since it's mostly just guard code against changes in TG affecting TGD in unexpected ways.

I'll be going back to converting more test fixtures.

This might be a reasonable merge point, if you like.

dodexahedron commented 8 months ago

I've adapted the KeyMap to fix things. Since it's a configuration class, and Key knows what to do with itself, the solution was remarkably simple: just make the Keys in KeyMap strings instead and then update usages.

Tests are now passing once again.

Relevant to #268 as well

dodexahedron commented 8 months ago

Just realized this PR was pointing at the older branch. Fast-forwarded it to sync up with convert-unit-tests-to-constraint-model-2, so this is now the same as was intended.

tznind commented 8 months ago

Great stuff! Thanks for all this work <3

dodexahedron commented 8 months ago

My pleasure.

I fast-forwarded my v2 fork to current and am now working on the convert-unit-tests-to-constraint-model-3 branch, from there.

I'll open a new PR. Rinse and repeat. 😅