_PathMake() issues in <File.au3>

[Synix <cross.fire@…>]

_PathMake() adds no backslash if the dir parameter is empty (only drive + filename set).
Also there are some other minor inconsistencies as you will see in my example:

#include <File.au3>

ConsoleWrite(@CRLF & "! " & _PathMake("c", "", "testfile", "ext")) ; no slash :(
ConsoleWrite(@CRLF & "- " & _PathMake("", "testfolder", "testfile", "ext")) ; adding leading slash is incorrect behavior on no drive
ConsoleWrite(@CRLF & "- " & _PathMake("", "testfolder", "", "")) ; adding trailing slash is incorrect behavior on no file
ConsoleWrite(@CRLF & "- " & _PathMake("c", "testfolder\\testfolder2", "testfile", "ext")) ; double slash :/
ConsoleWrite(@CRLF & "- " & _PathMake("c", "testfolder/testfolder2", "testfile", "ext")) ; wrong slash :/
ConsoleWrite(@CRLF & @CRLF)

This would be my fix for it:

Func _PathMake($sDrive, $sDir, $sFileName, $sExtension)
	; Format $sDrive, if it's not a UNC server name, then just get the drive letter and add a colon
	If StringLen($sDrive) Then
		If Not (StringLeft($sDrive, 2) = "\\") Then $sDrive = StringLeft($sDrive, 1) & ":"
	EndIf

	; Format the directory
	If StringLen($sDir) Then
		; Replace any [repeating] [back-]slashes with a single backslash
		$sDir = StringRegExpReplace($sDir, "[/\\]+", "\\")
		; Add leading backslash if drive is set
		If StringLen($sDrive) And Not (StringLeft($sDir, 1) = "\") Then $sDir = "\" & $sDir
	EndIf

	; Add trailing backslash to the directory if necessary
	If StringLen($sFileName) And (StringLen($sDrive) Or StringLen($sDir)) Then
		If Not (StringRight($sDir, 1) = "\") Then $sDir = $sDir & "\"
	EndIf

	; Nothing to be done for the filename

	; Add the period to the extension if necessary
	If StringLen($sExtension) Then
		If Not (StringLeft($sExtension, 1) = ".") Then $sExtension = "." & $sExtension
	EndIf

	Return $sDrive & $sDir & $sFileName & $sExtension
EndFunc   ;==>_PathMake

Surely you could improve the code even more (e.g. removing unnecessary preexisting leading/trailing slashes from the parameters), but I think you kept the function pretty minimalistic for efficiency reasons. That's why I hope you won't throw out the RegExpReplace from my suggestion code since it's not necessary for fixing the only real bug from my example (line 3), but it does way improve consistency.

[BrewManNH]

According to the help file, the second example you posted has the correct output. If the directory doesn't have a leading or a trailing slash, it adds it.

The third is exactly the same as the second, exactly as the help file states will happen.

The fourth and fifth examples are also correct per the help file. The help file states that This doesn't check the validity of the path created, it could contain characters which are invalid on your filesystem.

I'm not sure what the expected output for the first one should be, but according to how the function is written, it is also correct.

I'm going to change this to a feature request, because there is no bug here.

[Synix <cross.fire@…>]

Yes you are right, examples 2 to 5 are more of a feature request.
But the first one is a bug if you consider the remark of the help file text: 'You may pass "" (an empty string) for any part of the path you do not need in the final output.'
If I want to get the path of <Drive>:\mylogfile.log it won't add the needed backslash for the path to be a "path", since the function only does that for $sDir, which would be blank in this case.
The blank $sDir should automatically &= "\" if it's empty. This would perfectly fit the parameter description.

[BrewManNH]

The thing is, this function is practically useless, you have to pass it all of the parameters you already have, and you could easily create your own path from them.

I can't honestly say that I've ever seen a script use this function in the 4+ years I've been using AutoIt. But, whether it even needs to exist or not isn't a problem, and whether it should be rewritten or dumped all together, I'll leave to others.

[Synix <cross.fire@…>]

I've neither seen anyone using this function ever. Personally I think it's because of the uselessnes (no big checks or path fixes) and unflexibility of the function
This is why I wrote a _PathMakeEx() for my needs, which always outputs consistent path strings:

Func _PathMakeEx($sBaseDir, $sSubDir = "", $sFileName = "")
	If StringLen($sBaseDir) Then
		;replace any [repeating] [back-]slashes with a single backslash
		$sBaseDir = StringRegExpReplace(StringLeft($sBaseDir, 1), "[/\\]", "\\") & StringRegExpReplace(StringMid($sBaseDir, 2), "[/\\]+", "\\") ;keep potential double slashes at start
		;remove trailing backslash
		If StringLen($sBaseDir) >= 3 And StringRight($sBaseDir, 1) = "\" Then $sBaseDir = StringTrimRight($sBaseDir, 1)
		;remove leading backslash
		If StringLeft($sBaseDir, 1) = "\" And StringLeft($sBaseDir, 2) <> "\\" Then $sBaseDir = StringTrimLeft($sBaseDir, 1)
	EndIf
	If StringLen($sSubDir) Then
		;replace any [repeating] [back-]slashes with a single backslash
		$sSubDir = StringRegExpReplace($sSubDir, "[/\\]+", "\\")
		;remove trailing backslash
		If StringRight($sSubDir, 1) = "\" Then $sSubDir = StringTrimRight($sSubDir, 1)
		;add leading backslash if drive is set, remove if not
		If StringLen($sBaseDir) Then
			If StringLeft($sSubDir, 1) <> "\" Then $sSubDir = "\" & $sSubDir
		Else
			If StringLeft($sSubDir, 1) = "\" Then $sSubDir = StringTrimLeft($sSubDir, 1)
		EndIf
	EndIf
	If StringLen($sFileName) Then
		;replace any [repeating] [back-]slashes with a single backslash
		$sFileName = StringRegExpReplace($sFileName, "[/\\]+", "\\")
		;remove trailing backslash
		If StringRight($sFileName, 1) = "\" Then $sFileName = StringTrimRight($sFileName, 1)
		;add leading backslash if drive or sub dir is set, remove if not
		If StringLen($sBaseDir) Or StringLen($sSubDir) Then
			If StringLeft($sFileName, 1) <> "\" Then $sFileName = "\" & $sFileName
		Else
			If StringLeft($sFileName, 1) = "\" Then $sFileName = StringTrimLeft($sFileName, 1)
		EndIf
	EndIf

	Return $sBaseDir & $sSubDir & $sFileName
EndFunc   ;==>_PathMake

So I wouldn't even mind if the original _PathMake got dumped.

[guinness]

Your code suggestion is using an old version of the function.

[Synix <cross.fire@…>]

My suggestion from the initial post was based on the code in <File.au3> of AutoIt v3.3.12.0, even in beta v3.3.13.19 is no different version included.

This is now directly taken from the beta:

; #FUNCTION# ====================================================================================================================
; Author ........: Valik
; Modified.......: guinness
; ===============================================================================================================================
Func _PathMake($sDrive, $sDir, $sFileName, $sExtension)
	; Format $sDrive, if it's not a UNC server name, then just get the drive letter and add a colon
	If StringLen($sDrive) Then
		If Not (StringLeft($sDrive, 2) = "\\") Then $sDrive = StringLeft($sDrive, 1) & ":"
	EndIf

	; Format the directory by adding any necessary slashes
	If StringLen($sDir) Then
		If Not (StringRight($sDir, 1) = "\") And Not (StringRight($sDir, 1) = "/") Then $sDir = $sDir & "\"
	EndIf

	If StringLen($sDir) Then
		; Append a backslash to the start of the directory if required
		If Not (StringLeft($sDir, 1) = "\") And Not (StringLeft($sDir, 1) = "/") Then $sDir = "\" & $sDir
	EndIf

	; Nothing to be done for the filename

	; Add the period to the extension if necessary
	If StringLen($sExtension) Then
		If Not (StringLeft($sExtension, 1) = ".") Then $sExtension = "." & $sExtension
	EndIf

	Return $sDrive & $sDir & $sFileName & $sExtension
EndFunc   ;==>_PathMake

[guinness]

Since the first line is a bug, I am reverting back to a bug and fixing with this code. As for your feature request, it's outside the scope of the function and thus I am rejecting it. As this is something you can implement yourself I see no reason to add a script breaking change.

[guinness]

Fixed by revision [11102] in version: 3.3.13.20

[TicketCleanup]

Automatic ticket cleanup.