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.

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

[Xaml[C]] supports 'using:' xmlns declarations #851

Merged
merged 2 commits into from Apr 6, 2017

Conversation

Projects
None yet
5 participants
@StephaneDelcroix
Member

StephaneDelcroix commented Mar 31, 2017

Description of Change

[Xaml[C]] supports 'using:' xmlns declarations

	xmlns:local="using:Xamarin.Forms.Xaml.UnitTests"

This behaves exactly like clr-namespace without any other parameters

Bugs Fixed

/

API Changes

/

Behavioral Changes

/

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
if (xmlns.StartsWith("using:", StringComparison.Ordinal)) {
ParseUsing(xmlns, out typeName, out ns, out asm, out targetPlatform);
return;

This comment has been minimized.

@samhouts

samhouts Mar 31, 2017

Member

Just curious...why not do else?

@samhouts

samhouts Mar 31, 2017

Member

Just curious...why not do else?

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 31, 2017

Member

avoid unnecessary nesting ? no real reason for one option or the other. If you think I did that to optimize the compiled code, well, you're thinking too much :)

@StephaneDelcroix

StephaneDelcroix Mar 31, 2017

Member

avoid unnecessary nesting ? no real reason for one option or the other. If you think I did that to optimize the compiled code, well, you're thinking too much :)

foreach (var decl in xmlns.Split(';')) {
if (decl.StartsWith("using:", StringComparison.Ordinal)) {
ns = decl.Substring(6, decl.Length - 6);
continue;

This comment has been minimized.

@samhouts

samhouts Mar 31, 2017

Member

This doesn't seem useful.

@samhouts

samhouts Mar 31, 2017

Member

This doesn't seem useful.

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 31, 2017

Member

for the same reason stated before. to keep in sync, and be future-ish proof

@StephaneDelcroix

StephaneDelcroix Mar 31, 2017

Member

for the same reason stated before. to keep in sync, and be future-ish proof

This comment has been minimized.

@samhouts

samhouts Mar 31, 2017

Member

fair enough. the continue doesn't hurt anything, just seemed superfluous at this time. :)

@samhouts

samhouts Mar 31, 2017

Member

fair enough. the continue doesn't hurt anything, just seemed superfluous at this time. :)

{
typeName = ns = asm = targetPlatform = null;
foreach (var decl in xmlns.Split(';')) {

This comment has been minimized.

@samhouts

samhouts Mar 31, 2017

Member

So if the user does something silly, like include more than one using, then the last one wins. Is this something we need to have a test for to establish the behavior as expected?

@samhouts

samhouts Mar 31, 2017

Member

So if the user does something silly, like include more than one using, then the last one wins. Is this something we need to have a test for to establish the behavior as expected?

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 31, 2017

Member

silly user gets silly results.

I left the foreach, like in cld-namespace, because at some point in the future someone will surely asks for supporting targetPlatform

@StephaneDelcroix

StephaneDelcroix Mar 31, 2017

Member

silly user gets silly results.

I left the foreach, like in cld-namespace, because at some point in the future someone will surely asks for supporting targetPlatform

@StephaneDelcroix

This comment has been minimized.

Show comment
Hide comment
@StephaneDelcroix

StephaneDelcroix Apr 4, 2017

Member

@jassmith do you think this should support (as of NOW with the goal we have) targetPlatform ?

Member

StephaneDelcroix commented Apr 4, 2017

@jassmith do you think this should support (as of NOW with the goal we have) targetPlatform ?

@rmarinho rmarinho merged commit ab0faba into master Apr 6, 2017

@rmarinho rmarinho deleted the XmlnsUsing branch Apr 26, 2017

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts added this to the 2.3.5 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment