-
-
Notifications
You must be signed in to change notification settings - Fork 108
docs: clarify which generators use the attribute syntax #2510
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🔴 The new lead paragraph overstates what three of the four "property-aware" generators accept, contradicting the per-subcommand Synopsis sections later on the same page. (1)
generate propertyonly takes a singleproperty:typeand ignores all association flags —generatePropertyincli/lucli/Module.cfc:2692-2740reads onlyargs[1]/args[2]and never callsparseGeneratorArgs. (2)generate scaffoldandgenerate api-resourcesilently drop--hasOne=; the call sites atModule.cfc:2592-2598and2768-2773forward onlybelongsToandhasMany, andScaffold.cfc:25-33/362-369don't even define ahasOneparameter. Suggest splittingpropertyout (singlename:type, no flags) and noting that onlymodelhonors--hasOne=whilescaffold/api-resourceaccept--belongsTo=and--hasMany=only.Extended reasoning...
What the bug is
The PR's new lead under
## Attribute syntaxreads:This is a single, blanket claim about all four generators. It is wrong in two distinct ways, and it contradicts the Synopsis blocks lower in the same file.
Why it's wrong —
propertygeneratePropertyincli/lucli/Module.cfc:2692-2740only readsargs[1](model name) andargs[2](a singleproperty:type), splits the second on:, and usesparts[1]/parts[2]. There is no loop over additional positional args, noparseGeneratorArgs()call, and no--belongsTo/--hasMany/--hasOnerecognition.parseGeneratorArgsis only called fromgenerateModel(2437),generateScaffold(2589), andgenerateApiResource(2765). The doc's own Synopsis confirms this —wheels generate property <ModelName> <property:type>(singular, no flags) — and the example passes exactly one property.Why it's wrong —
scaffoldandapi-resourceon--hasOneparseGeneratorArgs(Module.cfc:4798) does collect ahasOnearray, but the scaffold call site atModule.cfc:2592-2598and the api-resource call site atModule.cfc:2768-2773forward onlybelongsToandhasManyto the service. The receiving signatures confirm this:services/Scaffold.cfc:25-33(generateScaffold(name, properties, belongsTo, hasMany, api, tests, force)) and362-369(generateApiResource(name, properties, belongsTo, hasMany, tests, force)) have nohasOneparameter. OnlygenerateModel(Module.cfc:2447-2453) wireshasOnethrough. The per-subcommand Synopsis sections for scaffold and api-resource correctly omit--hasOne=, so the new lead also contradicts the rest of the page.Impact
The PR's stated goal is to remove ambiguity by naming exactly which generators accept the attribute syntax. Instead it introduces a fresh, easily-checkable inaccuracy. A reader following the new lead and running:
…gets a single
email:stringmigration withphone:stringand--belongsTo=orgsilently discarded. Likewise:…generates the resource with no
hasOneassociation — no error, no warning. That is the same kind of confusion the PR sets out to fix.Step-by-step proof for
generate propertyproperty… accept[s] a trailing list ofname:typeproperty pairs… also recognise--belongsTo=,--hasMany=,--hasOne=."wheels generate property User a:string b:string --belongsTo=org.Module.cfc:2692generatePropertyenters; readsargs[1] = "User"andargs[2] = "a:string". Splitsa:stringon:→parts[1]=a,parts[2]=string.args[3..];b:stringand--belongsTo=orgare never inspected.aand prints success. The user's other inputs are gone.Step-by-step proof for
scaffold --hasOnewheels generate scaffold Post --hasOne=profile.Module.cfc:2589callsparseGeneratorArgs(args). Result includesparsed.hasOne = ["profile"].Module.cfc:2592-2598invokesscaffold.generateScaffold(name="Post", properties=…, belongsTo=…, hasMany=…, …).parsed.hasOneis never passed.Scaffold.cfc:25generateScaffoldhas nohasOneparameter to receive it. The value is gone.Post.cfccontains nohasOne(name="profile")line; no warning is printed.Suggested fix
Split the lead into something like:
That keeps the goal of the PR (naming which generators take the attribute syntax) while matching what the implementation actually does and the Synopsis sections below.