Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Annotate attributes with resource category hint for the IDE #2866

Merged
merged 12 commits into from
Mar 25, 2019
Merged

Annotate attributes with resource category hint for the IDE #2866

merged 12 commits into from
Mar 25, 2019

Conversation

kzu
Copy link
Contributor

@kzu kzu commented Mar 22, 2019

The code completion experience can be improved if the editor knew
which string properties typically come from resources so a completion
suggestion can be provided that is filtered to the type of resource
the property should be pointing to (i.e. @(string) vs @(drawable)).

Fixes #2862

@kzu kzu requested a review from jonpryor as a code owner March 22, 2019 18:45
@kzu
Annotate attributes with resource category hint for the IDE
ba158e9 
The code completion experience can be improved if the editor knew
which string properties typically come from resources so a completion
suggestion can be provided that is filtered to the type of resource
the property should be pointing to (i.e. `@(string)` vs `@(drawable)`).

Fixes dotnet#2862
@jonpryor
Copy link
Member

The macOS PR Release Pipeline Build is failing:

xamarin-android/src/Xamarin.Android.NamingCustomAttributes/Android.App/InstrumentationAttribute.cs(23,4): error CS0246: The type or namespace name 'Category' could not be found (are you missing a using directive or an assembly reference?)

Looks like using System.ComponentModel wasn't consistently inserted.

@jonpryor jonpryor merged commit d548ec9 into dotnet:master Mar 25, 2019
jonpryor pushed a commit that referenced this pull request Apr 3, 2019
@kzu @jonpryor
[Mono.Android] Add [Category] for IDE completion of string props (#2866)
a3d4ade 
Fixes: #2862

The code completion experience within Visual Studio can be improved
if the editor knows which string properties typically come from
resources so a completion suggestion can be provided that is filtered
to the type of resource the property should be pointing to
(i.e. `@(string)` vs `@(drawable)`).

Add a [`CategoryAttribute`][0] to various string properties,
providing a `;`-separated list of Android `@resource` types that
the Visual Studio IDE can key completion suggestions off of.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.categoryattribute?view=netframework-4.7.2
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Dec 4, 2020
@jonathanpeppers
Revert "[Mono.Android] Add [Category] for IDE completion of string pr…
29fc406 
…ops (dotnet#2866)"

Fixes: dotnet#5373

This reverts commit d548ec9.

Decoration of `ApplicationAttribute.Label` with:

    [System.ComponentModel.Category ("@string")]
    public string?                Label                   {get; set;}

Allowed an experimental feature in Visual Studio to offer `@string`
`AndroidResource` completion when changing the `Label` property in C#.

Unfortunately, the presence of this attribute causes the linker to
preserve various dependencies of `System.ComponentModel` in .NET 6.

It appears that this completion feature never shipped in the IDE, so
we can completely remove this. If we ever want to bring the feature
back, we should create a new `internal` attribute to be used instead.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Dec 7, 2020
@jonathanpeppers
Revert "[Mono.Android] Add [Category] for IDE completion of string pr…
1c8f2de 
…ops (dotnet#2866)"

Fixes: dotnet#5373

This reverts commit d548ec9.

Decoration of `ApplicationAttribute.Label` with:

    [System.ComponentModel.Category ("@string")]
    public string?                Label                   {get; set;}

Allowed an experimental feature in Visual Studio to offer `@string`
`AndroidResource` completion when changing the `Label` property in C#.

Unfortunately, the presence of this attribute causes the linker to
preserve various dependencies of `System.ComponentModel` in .NET 6.

It appears that this completion feature never shipped in the IDE, so
we can completely remove this. If we ever want to bring the feature
back, we should create a new `internal` attribute to be used instead.

Results of this change:

    > apkdiff before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
    -          56 lib/armeabi-v7a/libxamarin-app.so
    Section size difference
    -           8 .rel.dyn
    -          12 .data
    -       6,144 .bss
    -          77 assemblies/Mono.Android.dll
    -       3,194 assemblies/System.ComponentModel.Primitives.dll *1
    Summary:
    +           0 Other entries 0.00% (of 6,484)
    +           0 Dalvik executables 0.00% (of 317,508)
    -       3,271 Assemblies -0.15% (of 2,139,234)
    -          56 Shared libraries -0.00% (of 20,674,428)
    -       3,468 Package size difference -0.04% (of 9,485,287)
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Dec 8, 2020
@jonathanpeppers
Revert "[Mono.Android] Add [Category] for IDE completion of string pr…
58ba3a2 
…ops (dotnet#2866)"

Fixes: dotnet#5373

This reverts commit d548ec9.

Decoration of `ApplicationAttribute.Label` with:

    [System.ComponentModel.Category ("@string")]
    public string?                Label                   {get; set;}

Allowed an experimental feature in Visual Studio to offer `@string`
`AndroidResource` completion when changing the `Label` property in C#.

Unfortunately, the presence of this attribute causes the linker to
preserve various dependencies of `System.ComponentModel` in .NET 6.

It appears that this completion feature never shipped in the IDE, so
we can completely remove this. If we ever want to bring the feature
back, we should create a new `internal` attribute to be used instead.

Results of this change:

    > apkdiff before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
    -          56 lib/armeabi-v7a/libxamarin-app.so
    Section size difference
    -           8 .rel.dyn
    -          12 .data
    -       6,144 .bss
    -          77 assemblies/Mono.Android.dll
    -       3,194 assemblies/System.ComponentModel.Primitives.dll *1
    Summary:
    +           0 Other entries 0.00% (of 6,484)
    +           0 Dalvik executables 0.00% (of 317,508)
    -       3,271 Assemblies -0.15% (of 2,139,234)
    -          56 Shared libraries -0.00% (of 20,674,428)
    -       3,468 Package size difference -0.04% (of 9,485,287)
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Dec 8, 2020
@jonathanpeppers
Revert "[Mono.Android] Add [Category] for IDE completion of string pr…
fadbce9 
…ops (dotnet#2866)"

Fixes: dotnet#5373
Reverts: d548ec9

Decoration of `ApplicationAttribute.Label` with:

    [System.ComponentModel.Category ("@string")]
    public string?                Label                   {get; set;}

Allowed an experimental feature in Visual Studio to offer `@string`
`AndroidResource` completion when changing the `Label` property in C#.

Unfortunately, the presence of this attribute causes the linker to
preserve various dependencies of `System.ComponentModel` in .NET 6.

It appears that this completion feature never shipped in the IDE, so
we can completely remove this. If we ever want to bring the feature
back, we should create a new `internal` attribute to be used instead.

Results of this change:

    > apkdiff before.apk after.apk
    Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
    -          56 lib/armeabi-v7a/libxamarin-app.so
    Section size difference
    -           8 .rel.dyn
    -          12 .data
    -       6,144 .bss
    -          77 assemblies/Mono.Android.dll
    -       3,194 assemblies/System.ComponentModel.Primitives.dll *1
    Summary:
    +           0 Other entries 0.00% (of 6,484)
    +           0 Dalvik executables 0.00% (of 317,508)
    -       3,271 Assemblies -0.15% (of 2,139,234)
    -          56 Shared libraries -0.00% (of 20,674,428)
    -       3,468 Package size difference -0.04% (of 9,485,287)
jonpryor pushed a commit that referenced this pull request Dec 9, 2020
@jonathanpeppers
Revert "[Mono.Android] Add [Category] for IDE completion of string pr…
e5a4bee 
…ops (#2866)" (#5375)

Fixes: #5373

Reverts: d548ec9
Reverts: #2862

The idea in Issue #2862 and commit d548ec9 was to to improve code
completion for Android Resources within Visual Studio:

![Android Resource completion in Visual Studio][0]

This was achieved by adding [`CategoryAttribute`][1] to various
properties so that Visual Studio could know what completions to show:

	partial class ActivityAttribute {
	    [System.ComponentModel.Category ("@string")]
	    public string?                Label                   {get; set;}
	}

Unfortunately, the presence of `CategoryAttribute` causes the .NET 6
linker to preserve various dependencies of `System.ComponentModel`,
causing `.apk` files to be larger than desired.

Furthermore, the feature was *experimental*, and never shipped in a
stable Visual Studio release.

For now, revert commit d548ec9, removing the `[Category]` custom
attributes.  This will allow `.apk` files to be smaller for .NET 6.

Should we want to bring this feature back, we'll either need to see
if the linker can be improved to remove `CategoryAttribute`, or
instead introduce a set of `internal` attributes for this purpose.

Results of this change in a .NET 6 `.apk`:

	> apkdiff before.apk after.apk
	Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
	-          56 lib/armeabi-v7a/libxamarin-app.so
	Section size difference
	-           8 .rel.dyn
	-          12 .data
	-       6,144 .bss
	-          77 assemblies/Mono.Android.dll
	-       3,194 assemblies/System.ComponentModel.Primitives.dll *1
	Summary:
	+           0 Other entries 0.00% (of 6,484)
	+           0 Dalvik executables 0.00% (of 317,508)
	-       3,271 Assemblies -0.15% (of 2,139,234)
	-          56 Shared libraries -0.00% (of 20,674,428)
	-       3,468 Package size difference -0.04% (of 9,485,287)

[0]: https://user-images.githubusercontent.com/169707/54777844-f9b37500-4bf1-11e9-8e52-63982b8b362f.gif
[1]: https://docs.microsoft.com/dotnet/api/system.componentmodel.categoryattribute?view=netframework-4.7.2
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dellis1972 dellis1972 dellis1972 approved these changes

@jonpryor jonpryor Awaiting requested review from jonpryor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kzu @jonpryor @dellis1972