WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 5 months ago

Last modified 4 months ago

#38176 closed enhancement (fixed)

Add 'default' to register_setting

Reported by: rmccue Owned by: joehoyle
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords: has-patch dev-feedback needs-dev-note needs-refresh
Focuses: Cc:

Description

Imagine a future where you don't need to pass your default settings every time you call get_option. Instead, you simply pass into register_setting once.

Attached patch does this. Also moves the register_setting group of functions from wp-admin to wp-includes/option.php to allow usage everywhere, rather than just in the admin. This is probably a Good Idea with the changes made in #37885, as register_setting is now also useful for the REST API, not just the admin.

This does do some funky meta-programming that sucks a bit: if you call get_option( 'option_name' ) without a default parameter, it uses the registered default. If you call get_option( 'option_name', false ), it uses the value you passed in (false) even though this is the default parameter value. This isn't great, but allows us to do this nicely with backwards compatibility and is somewhat less surprising.

Attachments (5)

38176.diff (12.4 KB) - added by rmccue 6 months ago.
38176.2.diff (12.7 KB) - added by rmccue 6 months ago.
Add parameter to missed filter
38176.3.diff (3.5 KB) - added by jtsternberg 5 months ago.
Refreshed patch (now that functions have been moved to different file)
38176.4.diff (4.4 KB) - added by jorbin 5 months ago.
38176.5.diff (579 bytes) - added by jeremyfelt 5 months ago.

Download all attachments as: .zip

Change History (28)

@rmccue
6 months ago

#1 @rmccue
6 months ago

As an example of how this works:

<?php

// Register the setting.
register_setting( 'rmccue', 'rmccue_some_setting', [
        'default' => 'yolo',
]);

var_dump( get_option( 'rmccue_some_setting' ) );
// => 'yolo'

var_dump( get_option( 'rmccue_some_setting', false ) );
// => false

var_dump( get_option( 'rmccue_some_setting', null ) );
// => null

#2 @joehoyle
6 months ago

This looks good to me, +1 on the moving of the function too - right now I'm having to manually include the admin file when we want to use register_setting outside of the admin, which is fairly common now with the additions.

@rmccue
6 months ago

Add parameter to missed filter

This ticket was mentioned in Slack in #core by rmccue. View the logs.


6 months ago

#4 @joehoyle
6 months ago

  • Keywords dev-feedback added
  • Milestone changed from Awaiting Review to 4.7

#5 @rmccue
6 months ago

  • Keywords needs-dev-note added

This ticket was mentioned in Slack in #core by rmccue. View the logs.


6 months ago

#7 @rmccue
6 months ago

Going to move the settings functions in #37885 instead of here.

#8 @jorbin
6 months ago

  • Keywords needs-refresh added

With the moving of the functions done in [38687], this needs a refresh.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#10 @joehoyle
5 months ago

  • Owner set to joehoyle
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core by desrosj. View the logs.


5 months ago

#12 @joehoyle
5 months ago

I somewhat interesting and controversial idea suggested by @jorbin is to _deprecate_ the $default argument for get_option() and instead recommend the use of register_setting, as in (I think) virtually all cases, one would want a consistent default for their options, and it leads to developer error by having to re-specify the default on every get_option call.

I think this would be a pretty good idea, however is a small but wide impacting change, and I'm not sure that going this far is required just to support the default argument in register_setting. How about we start with adding it to register_setting but leave the deprecation of default in get_option for down the road?

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

@jtsternberg
5 months ago

Refreshed patch (now that functions have been moved to different file)

This ticket was mentioned in Slack in #core-restapi by joehoyle. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


5 months ago

@jorbin
5 months ago

#17 @joehoyle
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38910:

Options: Add 'default' to register_setting

Add a default argument to register_setting that will be used an the default option value viet get_option() in the event of no other option being specified. This means (if chosen) developers can define their default once via register_option and not have to duplicate the value every time they make a call to get_option().

Props rmccue, jorbin, jtsternberg.
Fixes #38176.

#18 @jeremyfelt
5 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The default_option_ filter in update_option() does not pass a third parameter, which the filter_default_option() function requires. See https://travis-ci.org/aaronjorbin/develop.wordpress/builds/170519570

@jeremyfelt
5 months ago

#19 @jeremyfelt
5 months ago

38176.5.diff adds false as the third parameter when applying the default_option_ filter in update_option(). This seems like the right assumption to make here.

This resolves the errors in the tests.

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


5 months ago

#21 @jeremyfelt
5 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 38916:

Options: Make $passed_default available in remaining default_option_{$option} filter.

Pass false as the $passed_default value when the default_option_{$option} filter is applied in update_option as no default is ever passed.

This resolves an error in tests where the 3rd parameter is not available to filter_default_option().

Fixes #38176.

#22 @ocean90
4 months ago

In 39382:

Options: Pass the $passed_default parameter to the 'default_option_{$option} filter in add_option().

This was missed in [38910].

Props joehoyle, lucasstark.
See #38176.
Fixes #38930.

#23 @ocean90
4 months ago

In 39383:

Options: Pass the $passed_default parameter to the default_option_{$option} filter in add_option().

This was missed in [38910].

Merge of [39382] to the 4.7 branch.

Props joehoyle, lucasstark.
See #38176, #38930.

Note: See TracTickets for help on using tickets.