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 attribute members with resource completion hint #2862

Closed
kzu opened this issue Mar 21, 2019 · 4 comments
Closed

Annotate attribute members with resource completion hint #2862

kzu opened this issue Mar 21, 2019 · 4 comments
Assignees
@jonpryor @kzu
Labels
Area: App Runtime Issues in `libmonodroid.so`. enhancement Proposed change to current functionality.
Milestone
d16-1

Comments

@kzu
Copy link
Contributor

kzu commented Mar 21, 2019 •
edited
Loading

As part of an exploration on how to improve the productivity of our code editing experience, we investigated improved code completion powered by Roslyn that could make it easier to use the myriad attributes where strings are intended to point to resource identifiers for various scenarios (i.e. ActivityAttribute)

The intended user experience would look like the following:

CompletionFromResource

In order to make the completion smart about what the various string values mean, I propose annotating the attribute properties with the existing [System.ComponentModel.Category("CATEGORY")] attribute that the IDE can then use to filter the offered values. To drive the above completion for the Activity.Label property, for example, we would annotate it with:

[Category("@string")]
public string Label {get; set;}

Any other attributes that point to any of the generated resource identifiers in Resource.designer.cs are potential candidates for being annotated this way too.

The associated user story on the IDE side is at https://devdiv.visualstudio.com/DevDiv/_workitems/edit/724024
@dellis1972
Copy link
Contributor

Would we be able to apply multiple Attributes for properties which can get source from multiple places ?
For example

[Category("@drawable")]
[Category("@mipmap")]
public string Icon {get; set;}

@kzu
Copy link
Contributor Author

kzu commented Mar 21, 2019 •
edited
Loading

well, I think in that case we could just use [Category("@drawable;@mipmap")].

@jonpryor
Copy link
Member

Behold, docs! CategoryAttribute

From which we can see that AttributeUsageAttribute.AllowMultiple is not set, which means it should have the default value of false, i.e. no, it cannot be specified multiple times.

We'll need to use @kzu's idea of using a ;-separated list of types.

kzu added a commit to kzu/xamarin-android that referenced this issue Mar 22, 2019
@kzu
Annotate attributes with resource category hint for the IDE
a909d69 
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
@kzu kzu mentioned this issue Mar 22, 2019
Merged
@kzu
Copy link
Contributor Author

kzu commented Mar 22, 2019

PR sent :)

kzu added a commit to kzu/xamarin-android that referenced this issue Mar 22, 2019
@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
@grendello grendello added enhancement Proposed change to current functionality. Area: App Runtime Issues in `libmonodroid.so`. labels Mar 25, 2019
@grendello grendello added this to the d16-1 milestone Mar 25, 2019
jonpryor pushed a commit that referenced this issue Mar 25, 2019
@kzu @jonpryor
[Mono.Android] Add [Category] for IDE completion of string props (#2866)
d548ec9 
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
jonpryor pushed a commit that referenced this issue 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 jonathanpeppers mentioned this issue Dec 4, 2020
Closed
jonpryor pushed a commit that referenced this issue 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
@ghost ghost locked as resolved and limited conversation to collaborators Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jonpryor jonpryor

@kzu kzu

Labels
Area: App Runtime Issues in `libmonodroid.so`. enhancement Proposed change to current functionality.
Projects
None yet
Milestone
d16-1
Development

No branches or pull requests
4 participants
@jonpryor @kzu @grendello @dellis1972