[wp-trac] [WordPress Trac] #31616: Splitting request_filesystem_credentials into separate functions

[WordPress Trac]

#31616: Splitting request_filesystem_credentials into separate functions
---------------------------------------+------------------
 Reporter:  jipmoors                   |       Owner:
     Type:  enhancement                |      Status:  new
 Priority:  normal                     |   Milestone:  4.2
Component:  Filesystem API             |     Version:
 Severity:  normal                     |  Resolution:
 Keywords:  shiny-updates needs-patch  |     Focuses:
---------------------------------------+------------------
Changes (by jorbin):

 * keywords:  has-patch => shiny-updates needs-patch
 * milestone:  Awaiting Review => 4.2


Comment:

 Thank Jimpoors.  I've take an initial look through your patch and have a
 couple of notes.  There might be more things that I missed, but overall
 this is a strong initial patch.

 Can you also talk through how you would handle the use cases we need this
 for. For example: What function (or functions) would you anticipate
 someone calling to find out if we need to ask user for filesystem
 credentials?

 get_filesystem_method -

 var_export isn't really needed for context since it's a string or
 allow_relaxed_file_ownership since it's a bool.  I would also move it down
 so that the initial ! $context check can run. I would put it above the (
 if ! $method ) I would also include  $method.

 The formatting for the docs on filesystem_method filter seems to have
 gotten screwed up.  Let's keep the alignment.

 Otherwsise, looks really good. I like that you are saving everything so we


 get_filesystem_credentials -

 request_filesystem_credentials filter docs formatting is screwed up


 While it's unlikely that anyone is causing the filtered value of
 request_filesystem_credentials to return a value that triggers empty
 instead of !=== '',  there are a number of values that would fit this bill
 such as
  •      0 (0 as an integer)
  •      0.0 (0 as a float)
  •      "0" (0 as a string)
  •      NULL
  •      FALSE
  •      array() (an empty array)
  •      $var; (a variable declared, but without a value)


 Therefore, let's keep the strict empty string check.


 request_filesystem_credentials -

 request_filesystem_credentials filter docs shouldn't be repeated.  The
 second one should reference the first one.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/31616#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform