FEAT: STYLIZE function and STYLES keyword

[rebolek]

This merge request implements stylize function and style VID keyword as available in Rebol 2. The syntax is same. It adds make-style function to VID object to reduce duplicated code.

[greggirwin]

We definitely want at least some manual tests, but we'll also see if @PeterWAWood has thoughts on automated tests for this.

Peter, do we have any areas or approaches used for semi-automated and manual tests? Not as good as fully automated, but better than nothing.

[hiiamboris]

I don't mind if you use some of the https://github.com/red/red/blob/master/tests/source/view/base-self-test.red functionality. The only reason I didn't split it from the tests is because I didn't have any GUI tests other than base yet. But that's the plan.

Maybe an overkill though... ☺

[PeterWAWood]

@greggirwin

Firstly, I don't think semi-automated or manual tests are useful for the regression testing approach that is used. Nobody will take the time to run or check them.

Secondly, little thought has been given to testing Red GUI. It is possible that the test GUI backend could provide, at least, the first level of automated testing. Unfortunately, I've never been able to successfully compile a program using it.

It should be possible to develop a testing approach based on generating an image of what is displayed and comparing it against expected images. There would need to be one expected image for each OS/GUI backend combination.

Perhaps @hiiamboris's work could be expanded on to come up with something more generic.

It would be a lot of work to develop a good approach to GUI testing. Personally, I think it would be a worthwhile investment as the GUI seems to be the source of many issues.

If something is needed quickly, Python AutoGUI could be usedto check the correctness of displays generated by Red GUI.

[rebolek]

@PeterWAWood Testing stylize doesn't actually need GUI at all. I think there just needs to be comparison if face objects generated by style keyword are same as those generated by stylize function. I'm working on some testing functions for it and will post them (probably in separate branch). I'll also take a look at @hiiamboris's base tests, if it's good fit for stylize.

[greggirwin]

@PeterWAWood manual tests aren't perfect for regression testing, but I don't know who we'll assign to write an automated GUI testing system. It will be a huge effort. My thought is that if there are at least some tests that can be run easily, if not automatically, should something change or break, it provides at least some baseline for comparison.

I'm all for full automation but, realistically, it's a low priority compared to many other things. Until then, what can we do (which is better than nothing)?

[PeterWAWood]

@PeterWAWood Testing stylize doesn't actually need GUI at all. I think there just needs to be comparison if face objects generated by style keyword are same as those generated by stylize function. I'm working on some testing functions for it and will post them (probably in separate branch).

@rebolek Thanks Bolek. (I really should have worked this out myself :-( )

[PeterWAWood]

@greggirwin My perception, based on what has happened in the past, is that manual regression tests rarely get run. When they do most people don't bother to check the results, they assume all is well if the test doesn't crash.

[PeterWAWood]

@greggirwin Here is a case in point. I ran the manual view-test.red against the latest master version. A screen shot is attached. I can see 8 errors. There could well be more, I'm not sure how the display should look.

[rebolek]

I'm working on tests in https://github.com/rebolek/red/tree/stylize-test branch. It's the system I described, It compares output of layout function with face! objects.

[rebolek]

@greggirwin @dockimbel While working on tests, I found one oddity. stylize returns block!, but stylize/master returns map!. It's because master styles in system/view/VID/styles are stored in map!, but local-styles in stylize and layout are block.

Functionally, it's same, but returning same type would be better IMO. I think there are four different possibilities, I'll list them from (IMO) lowest priority to highest:

1. leave it, as it is
2. change master stylesheet to block!
3. leave local-styles as block! and when needed, convert them to map! on return from stylize
4. turn local-styles to map!

What do you think?

[greggirwin]

I like that styles are a map, as that provides info on how the data is structured. Doubtful it's a performance win, given the small number of items in it. /styles css arg is also a block, as we may not have had map! in place when it was done, and that was inherited from R2. local-styles also uses reduce/into as an optimization, when adding new styles. But if we use maps:

					either pos: find local-styles name [pos/2: value][ 
						reduce/into [name value] tail local-styles
					]

becomes

					local-styles/:name: value

yes?

I vote for map! as the style structure all around. Though that means code changes and ripples to fetch-options as well, which is also spec'd to take a style/css block.

[rebolek]

@greggirwin Thanks for comment! I prefer map! too, keeping block! as supported type is possible, it should be basically free. I'll make required changes and post them to this branch in separate commit and @dockimbel can decide whether to accept it or no.

[rebolek]

Added new commit - styles are now always map! internally and stylize always returns map!.

NOTE: layout/styles and stylize/styles still accept block! as their css arg. It's very simple to support it, but if you find it confusing, I'll limit it just to map!.

[greggirwin]

It looks like the CI error was a timeout. If somebody backs me up on that, and this all looks good, I'll merge it.

[hiiamboris]

base-self-test hangs on my W7 with this PR, so this has to be debugged first

[rebolek]

@hiiamboris thanks for info, I'll check it.

[rebolek]

@hiiamboris Problem confirmed and found what causes it. Now I need to come up with some fix.