WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 3 months ago

#21156 new enhancement

Move utility functions out of wp-admin/network.php

Reported by: scribu Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords:
Focuses: multisite Cc:

Description

There are several functions in wp-admin/network.php that would come in handy outside the WP Admin -> Network Setup screen:

  • network_domain_check()
  • allow_subdirectory_install()
  • get_clean_basedomain()

etc.

They could be moved to wp-admin/includes/ms.php

Attachments (1)

21156.diff (4.2 KB) - added by jeremyfelt 9 months ago.
Moves utility functions to ms.php

Download all attachments as: .zip

Change History (8)

jeremyfelt9 months ago

Moves utility functions to ms.php

comment:1 jeremyfelt9 months ago

  • Keywords has-patch added; dev-feedback removed
  • Version set to 3.0

21156.diff moves network_domain_check(), allow_subdomain_install(), allow_subdirectory_install(), and get_clean_basedomain() from the display page of wp-admin/network.php to wp-admin/includes/ms.php. In the process, added inline docs for the global $wpdb objects.

comment:2 jeremyfelt8 months ago

  • Milestone changed from Awaiting Review to 3.7

comment:3 follow-up: nacin8 months ago

admin/includes/ms.php only gets included when multisite is enabled. As network.php is for converting to a network, ms.php cannot be relied upon. Including ms.php and allowing code from it to be executed before multisite is loaded could have nasty side effects.

I imagine scribu wants these functions specifically for WP-CLI, which is totally understandable. Beyond that, though, I'd really hesitate to make too much of network creation truly public, because most of these functions are either based on our rudimentary UI and upgrade process, or on our current subdirectory/subdomain restrictions. Long term, both get dropped. populate_network() already exists, which is sufficient for most use cases.

To satisfy WP-CLI's needs, I'd suggest pushing these functions to be usable with a few changes:

  • Renaming network_domain_check() and having it return *. Maybe ms_is_network_installed()
  • Probably finding a better name for get_clean_basedomain().

Definitely fine with allow_subdirectory_install() and allow_subdomain_install() to be public. Note they'll likely just return; or return true; at some later point when subdomain versus subdirectory goes away.

No need for the @var's, we don't really do those on reference, just initial defines. It'd also be @global anyway.

comment:4 in reply to: ↑ 3 jeremyfelt8 months ago

Replying to nacin:

No need for the @var's, we don't really do those on reference, just initial defines. It'd also be @global anyway.

Oops, global is good. We should do them on reference though, they come in extremely handy when reading core in an IDE.

I'll let scribu chime in on the rest.

comment:5 DrewAPicture8 months ago

  • Cc xoodrew@… added

comment:6 jeremyfelt7 months ago

  • Keywords has-patch removed
  • Milestone changed from 3.7 to Future Release

Punting to a future release. Looks like we can do a few things here, but more feedback and a patch is needed.

comment:7 jeremyfelt3 months ago

  • Component changed from Multisite to Networks and Sites
  • Focuses multisite added
Note: See TracTickets for help on using tickets.