-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
chore(dotnet): improve name generation for objects #5860
chore(dotnet): improve name generation for objects #5860
Conversation
utils/doclint/generateDotnetApi.js
Outdated
@@ -316,6 +326,8 @@ function generateNameDefault(member, name, t, parent) { | |||
if (member.kind === 'method' || member.kind === 'property') { | |||
// this should be easy to name... let's call it the same as the argument (eternal optimist) | |||
let probableName = `${parent.name}${translateMemberName(``, name, null)}`; | |||
if (specialNameResolutionMap.has(probableName)) |
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.
Like I mentioned in the chat it'd be nice if the list had only exceptions and most of the names would be generated in accord with some common rule (like ParamName or ClassParamName etc). While SelectOption
looks like an exception Position could probably be derived by a common rule. We'll need this mapping at least in one more place - playwright.dev generator
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.
Yeah, that's a valid point.
I've gone and improved this so the names now being generated follow a logical rule. The only "special case" I had to add was for names that are too generic, like value
, which gets translated to SelectOptionValue
. But still, I'm much happier with this iteration.
utils/doclint/generateDotnetApi.js
Outdated
} | ||
while (true) { | ||
// crude attempt at removing plurality | ||
if (attemptedName.endsWith('s')) |
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 is brittle, will fail on names like properties
. Perhaps whitelist known names?
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 is, to some extent... But also, we only have a few of these that are actually plural, so I can work around with something like
if (attemptedName.endsWith('s') && !attemptedName === 'properties')
:-)
When we add something that'll break this simple logic, I can always revisit this... It's the same if I have to add a map entry, or a check, right?
No description provided.