Skip to content

Clarify argument format in groupBy#6366

Closed
franzholz wants to merge 3 commits intoTYPO3-Documentation:mainfrom
franzholz:patch-64
Closed

Clarify argument format in groupBy#6366
franzholz wants to merge 3 commits intoTYPO3-Documentation:mainfrom
franzholz:patch-64

Conversation

@franzholz
Copy link
Contributor

It is quoted automatically.
The generation of the documentation needs an escape character.

See
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html

It is quoted automatically.
The generation of the documentation needs an escape character.

See
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html
Comment on lines 895 to 897
* Each argument is either a field name (e.g. in :sql:`GROUP BY \`bodytext\``),
a :php:`table.fieldName` or a :php:`tableAlias.fieldName`. And it gets properly
quoted automatically.
Copy link

@SophieBarlian SophieBarlian Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Each argument is either a field name (e.g. in :sql:`GROUP BY \`bodytext\``),
a :php:`table.fieldName` or a :php:`tableAlias.fieldName`. And it gets properly
quoted automatically.
* `$myGroupArray` can consist of one or several field names, separated by comma. It is possible and might be required to precede each field name with table name or table alias, for example :php:`table.fieldName` or :php:`tableAlias.fieldName`. In the resulting query fields are automatically properly quoted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your explanation is too long. It should be shorter. No need to explain the PHP syntax.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'd

Suggested change
* Each argument is either a field name (e.g. in :sql:`GROUP BY \`bodytext\``),
a :php:`table.fieldName` or a :php:`tableAlias.fieldName`. And it gets properly
quoted automatically.
* `Here one or several field names, separated by comma are added. It is possible to precede each field name with table name or table alias. In the resulting query fields are automatically properly quoted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the "separated by comma". PHP developers know this.
Remove the first sentence. It is a repetition of the line above.
The quoting sentence should be shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my changed texts.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's getting worse than the initial phrasing before your change. I tried to "save" your desire to improve here something but would rather vote to reject your commit then.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, saw your latest commit, I think it's ok like that.

@linawolf
Copy link
Member

linawolf commented Feb 3, 2026

Thanks for the contribution.

After review by the documentation team, we’ve decided not to proceed with this change. The current wording already meets our requirements for clarity and precision in reference documentation, and we don’t see a net improvement here.

We’re therefore closing this PR.

@linawolf linawolf closed this Feb 3, 2026
@franzholz
Copy link
Contributor Author

I just wanted to get almost the same description for groupBy as it is done some lines before for orderBy, because I think that this makes it easier to understand the meaning.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants