[wp-trac] [WordPress Trac] #22267: Add optional boolean argument to trailingslashit() and untrailingslashit()

[WordPress Trac]

#22267: Add optional boolean argument to trailingslashit() and untrailingslashit()
-----------------------------+-------------------------
 Reporter:  knutsp           |       Type:  enhancement
   Status:  new              |   Priority:  normal
Milestone:  Awaiting Review  |  Component:  Formatting
  Version:  2.2              |   Severity:  normal
 Keywords:  2nd-opinion      |
-----------------------------+-------------------------
 #20778 demonstrates that untrailinhslashit(), hence trailingslashit(), are
 unable to remove a trailing backslash from file paths on Windows.

 As Windows accepts a slash as directory separator, DIRECTORY_SEPARATOR is
 not needed to make portable code, except when parsing or exploding a path
 that comes from the system [http://alanhogan.com/tips/php/directory-
 separator-not-necessary]. We neeed to tell trailingslashit() and
 untrailingslashit() that we want to remove the trailing slash a file path
 from system, not a generated one and not a URL path.

 My idea is:
 {{{
 function untrailingslashit( $string, $is_path = false ) {
     if ( $is_path )
         return rtrim( $string, DIRECTORY_SEPARATOR . '/' );
     else
         return rtrim( $string, '/');
 }

 function trailingslashit( $string, $is_path = false ) {
     if ( $is_path )
         return untrailingslashit( $string, $is_path ) .
 DIRECTORY_SEPARATOR;
     else
         return untrailingslashit( $string ) . '/';
 }
 }}}

 Expected results that is true:

 {{{
   trailingslashit( '/mypath'  ) == '/mypath/'
   trailingslashit( '/mypath/' ) == '/mypath/'
   trailingslashit( '/mypath//') == '/mypath/'
 untrailingslashit( '/mypath/' ) == '/mypath'
 untrailingslashit( '/mypath//') == '/mypath'
   trailingslashit( '/usr/tmp/', true ) == '/usr/tmp/' // On non-Windows
   trailingslashit( '/usr/tmp',  true ) == '/usr/tmp/' // On non-Windows
   trailingslashit( '/usr/tmp//',true ) == '/usr/tmp/' // On non-Windows
   trailingslashit( 'C:\TEMP',   true ) == 'C:\TEMP\'  // On Windows
   trailingslashit( 'C://TEMP//',true ) == 'C://TEMP\' // On Windows
   trailingslashit( 'C:/TEMP/',  true ) == 'C:/TEMP\'  // On Windows
 untrailingslashit( 'C:\TEMP\',  true ) == 'C:\TEMP'   // On Windows
 }}}

 On Windows, for making a path to $subdir one may use either

 {{{
 $path = trailingslashit( $path, true ) . $subdir;
 $path = untrailingslashit( $path, true ) . '/' . $subdir;
 }}}
 according to preference in the situation, knowing that '/' is a valid
 separator in any case.

 The added DIRECTORY_SEPARATOR in trailingslashit() code suggestion above
 could be '/', but it might produce an unexpected result, some someone when
 deliberately wants to make a true native path for the system.

 The second parameter to rtrim() in untrailingslashit() should instruct
 rtrim() to remove both characters, as we cannot assume a Windows file path
 is always delimited or trailed with "\".

 I will make the patch, and will test the all the expected results on
 Windows and Linux. That is, if I am not convinced this is no way to go
 (adding optional parameters and "messing" with an old formatting
 function).

 == Alternatives ==

 We could make untrailingslashpath() and trailingslashpath() which assumes
 a path string, and no extra argument. This would require, at least softly,
 a replace all over the code, where a path is to be (un)trailingslashed.

 We could make untrailingbackslashit() and trailingbackslashit() instead,
 but this would force us to check either DIRECTORY_SEPARATOR or $is_win
 before selecting which function to use, bloating code all over the place.
 A good helper function should be a real help, especially in edge cases.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/22267>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software