-
Notifications
You must be signed in to change notification settings - Fork 413
First name and aliases cleanup #1990
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
Conversation
move GetLongestAlias to IdentifierSymbol and re-use where possible
Keboo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
| argument.FirstParent.Next is null) | ||
| { | ||
| var alias = identifierSymbol.Aliases.First(); | ||
| var alias = identifierSymbol.GetLongestAlias(removePrefix: false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't really getting the longest alias but the first one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was inconsistent. As you can see other places in the source code were using longest option alias to get its default alias. Here we were using the first one but it was a bug (at least in my opinion).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this just use the name rather than an alias?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Option.Name should not contain the prefix, while here we care about prefix.
Co-authored-by: David Cantú <[email protected]>
Symbolwere defining it's own name field. I've removed these and now all of them are just using_namedefined bySymbolOption.HasAliasIgnoringPrefixas it's not for general purposeGetLongestAliaspart of base type and re-using it where possible_aliasesfield, as it will most likely change once we introduce the concept of a hidden alias