tomohulk / WinSCP

WinSCP PowerShell Wrapper Module
GNU General Public License v3.0
153 stars 30 forks source link

Add $XmlLogPath parameter to allow for specifying a custom location for the temporary WinSCP XML log file. #155

Closed AndrewDP23 closed 1 year ago

AndrewDP23 commented 1 year ago

Added $XmlLogPath parameter to allow for specifying a custom location for the XML Log Path. This is a workaround for issues I am encountering where the default Windows temp path cannot be written to.

tomohulk commented 1 year ago

This is a good find, and thank you for sharing. However, the way you implemented it added some redundant code that isn't needed. If you would update your request, I will merge it.

  1. Please add some validation checking to your $XmlLogPath parameter (see the $SessionLogPath parameter for what I mean) to ensure if a custom path is provided, its a valid path.
  2. if you look at this code:
                # Enumerate each parameter.
                $sessionObjectProperties = $session |
                    Get-Member -MemberType Property |
                        Select-Object -ExpandProperty Name
                $keys = ($PSBoundParameters.Keys).Where({
                    $_ -in $sessionObjectProperties
                })

                foreach ($key in $keys) {
                    $session.$key = $PSBoundParameters.$key
                }

basically the code loops through every property on the WinSCP.Session object and maps them to the corresponding Function Parameter that is passed in the function call. Assuming the parameter name is the exact same as the property name on the object. So as long as the property name on a WinSCP.Session is XmlLogPath, then the code you added to map the property can be removed, and that block of code would add it automatically.

Thanks again for your request.

tomohulk commented 1 year ago

Also, diving into the WinSCP.Session object more, should add the $XmlLogPreserve Argument as well

image
tomohulk commented 1 year ago

@martinprikyl why aren't these XML log properties listed on the docs page for the WinSCP.Session object properties?

AndrewDP23 commented 1 year ago

Thanks for this @tomohulk, I've added the path validation to the $XmlLogPath parameter and have added the additional $XmlLogPreserve parameter for review.

Apologies for having three check-ins for this. Please let me know if you need me to cancel this and commit both the $XmlLogPath and validation script in a single check-in.

martinprikryl commented 1 year ago

@martinprikyl why aren't these XML log properties listed on the docs page for the WinSCP.Session object properties?

Not sure :) I'll document them.