Avalonia icon indicating copy to clipboard operation
Avalonia copied to clipboard

Add x:SetterTargetType

Open pr8x opened this issue 1 year ago • 14 comments

Adding x:SetterTargetType to allow for using <Setter> outside of <Style>.

Issues:

  • ERROR: XAMLIL Unable to find emitter for node type: XamlX.Ast.XamlAstXmlDirective Line 6, position 6. in D:\Repositories\Avalonia\src\Avalonia.Build.Tasks\bin\Debug\net6.0\MainWindow.axaml 6:6-6:6

    Other transformers seem to remove the XamlAstXmlDirective from the children collection. I tried doing that as well, but for some reason it processes the node twice and on the second run it will fail to find the attribute and fail the compilation.

    Repro: av_repro_SetterTargetType_emit_error.patch.txt

  • It seems that VS does not pick up this new attribute. How do I register it properly?

    image

CC: @grokys @kekekeks

Fix #6960

pr8x avatar Jul 22 '22 11:07 pr8x

You can test this PR using the following package version. 0.10.999-cibuild0022325-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

avaloniaui-team avatar Jul 22 '22 11:07 avaloniaui-team

It seems that VS does not pick up this new attribute. How do I register it properly?

Looks like a Resharper highlighting. Am I right? In this case JetBrains would need to support it.

Other transformers seem to remove the XamlAstXmlDirective from the children collection

I haven't checked other transformers, but from what I remember "x:" attributes are usually replaced (like Name or DataType) or ignored after reading. Compiler will remove them later anyway, so it should be safe to leave it.

Can you also add some tests in Markup.Xaml.Tests project? It also makes easier debugging of compiler.

maxkatz6 avatar Jul 23 '22 03:07 maxkatz6

Looks like a Resharper highlighting. Am I right

True, seems to be some ReSharper weirdness.

pr8x avatar Jul 25 '22 08:07 pr8x

You can test this PR using the following package version. 0.10.999-cibuild0022454-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

avaloniaui-team avatar Jul 25 '22 09:07 avaloniaui-team

@pr8x just curious on what's the supposed use case for this one?

jmacato avatar Jul 25 '22 11:07 jmacato

You can test this PR using the following package version. 0.10.999-cibuild0022466-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

avaloniaui-team avatar Jul 25 '22 11:07 avaloniaui-team

@jmacato Constructing <Animation> outside of <Styles> (e.g. inside Resources or for assigning to property)

pr8x avatar Jul 25 '22 14:07 pr8x

that's a good use case! thanks for the example!

jmacato avatar Jul 25 '22 15:07 jmacato

You can test this PR using the following package version. 0.10.999-cibuild0022486-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

avaloniaui-team avatar Jul 26 '22 09:07 avaloniaui-team

Removing the XamlAstXmlDirective works in the UT, but still fails when I try to do it in Sandbox (see repro patch in description). For some reason the transformer is called twice...

pr8x avatar Jul 26 '22 15:07 pr8x

@pr8x test fails if you copypaste this sandbox patch to the unit test. Except you need to remove "x:Class" part. Test also should be initialized with using (UnitTestApplication.Start(TestServices.StyledWindow)).

From what I checked, if fails when there are two or more setters in the animation. Sandbox code with single keyframe works fine.

maxkatz6 avatar Jul 27 '22 03:07 maxkatz6

@pr8x I think probably better way would be to one more transformer as well similar to AvaloniaXamlIlControlThemeTransformer, and replace Animation with AvaloniaXamlIlTargetTypeMetadataNode by filling ScopeTypes with a new enum value and TargetType. Compiler should process this node normally then, and it will be able to read TargetType from there for multiple setters. At least this way ControlTheme and ControlTemplate work around this problem.

maxkatz6 avatar Jul 27 '22 03:07 maxkatz6

@maxkatz6 I wonder why it tries to emit something for these directive nodes at all? Couldn't it just ignore them? Wouldn't we have the same issue for AvaloniaXamlIlTargetTypeMetadataNode that it tries to emit something it? Also feels weird to just replace Animation with a metadata node. What if in the future we want to add a transformer for Animation?

pr8x avatar Jul 27 '22 09:07 pr8x

What if in the future we want to add a transformer for Animation?

From what I understand, AvaloniaXamlIlTargetTypeMetadataNode special node with metadata just wraps original one. And original one is still available and can be transformed later. @kekekeks if you can confirm it.

maxkatz6 avatar Jul 28 '22 05:07 maxkatz6

@pr8x @kekekeks I added a new transformer to inject AvaloniaXamlIlTargetTypeMetadataNode with the parsed type. Please take a look.

maxkatz6 avatar Oct 24 '22 05:10 maxkatz6

You can test this PR using the following package version. 11.0.999-cibuild0025280-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

avaloniaui-team avatar Oct 24 '22 05:10 avaloniaui-team

You can test this PR using the following package version. 11.0.999-cibuild0025413-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

avaloniaui-team avatar Oct 26 '22 07:10 avaloniaui-team