incorrect uses of isRebuilding() in inline docs in processForm()

// If $form_state->isRebuilding() has been set and input has been // processed without validation errors, we are in a multi-step workflow // that is not yet complete. A new $form needs to be constructed based on // the changes made to $form_state during this request. Normally, a submit // handler sets $form_state->isRebuilding() if a fully executed form // requires another step. However, for forms that have not been fully // executed (e.g., Ajax submissions triggered by non-buttons), there is no // submit handler to set $form_state->isRebuilding(). It would not make // sense to redisplay the identical form without an error for the user to // correct, so we also rebuild error-free non-executed forms, regardless // of $form_state->isRebuilding().

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

@quietone opened merge request.
Status changed to Needs review 3 months ago7:40am 25 August 2024
Comment 3 months ago →
🇳🇿New Zealand quietone
Changed to use a phrase of the form, "'rebuild' has been set to TRUE", which is from the docs for \Drupal\Core\Form\FormState::$rebuild.
Status changed to Needs work 3 months ago12:02pm 25 August 2024
Comment 3 months ago →
🇬🇧United Kingdom joachim
> I don't think this an improvement, we're taking an awkwardly phrased comment about an API method and making it about a protected property instead.

Agreed -- the protected property is internal.

> If 'rebuild' has been set to TRUE

This doesn't help either. How do I find out more about what 'rebuild' is and how to set it or check for it?

> What about 'If $form_state->isRebuilding() returns TRUE" at least for the first part of the comment.

Maybe. It depends whether the reader of this documentation is trying to know

A. 'how do I know which behaviour I will get here' --> isRebuilding() is the key thing because that's how you find out
B. 'how do I influence which behaviour I will get here' --> isRebuilding() is one step removed from this AND its docs don't tell you how to change that status, and what you actually need to know about is setRebuild()

I'd lean towards B.