#2909 closed Bug (Fixed)
_PathMake() issues in <File.au3>
Reported by: | Synix <cross.fire@…> | Owned by: | guinness |
---|---|---|---|
Milestone: | 3.3.13.20 | Component: | AutoIt |
Version: | Other | Severity: | None |
Keywords: | Cc: |
Description
_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.
Attachments (0)
Change History (10)
comment:1 Changed 10 years ago by BrewManNH
- Type changed from Bug to Feature Request
- Version 3.3.12.0 deleted
comment:2 Changed 10 years ago by 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.
comment:3 Changed 10 years ago by 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.
comment:4 Changed 10 years ago by 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.
comment:5 Changed 10 years ago by guinness
Your code suggestion is using an old version of the function.
comment:6 Changed 10 years ago by 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
comment:7 Changed 10 years ago by guinness
- Type changed from Feature Request to Bug
comment:8 Changed 10 years ago by 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.
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 & "\" Else $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
comment:9 Changed 10 years ago by guinness
- Milestone set to 3.3.13.20
- Owner set to guinness
- Resolution set to Fixed
- Status changed from new to closed
Fixed by revision [11102] in version: 3.3.13.20
Guidelines for posting comments:
- You cannot re-open a ticket but you may still leave a comment if you have additional information to add.
- In-depth discussions should take place on the forum.
For more information see the full version of the ticket guidelines here.
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.