search

ppannuto / python-saleae

Python library to control a Saleae Logic Analyzer
Apache License 2.0
124 stars 55 forks source link

Windows str.replace not implemented properly #66

Open AndyEveritt opened 3 years ago

AndyEveritt commented 3 years ago

You often have a line such as this to fix windows file paths:

# Fix windows path if needed
file_path_on_target_machine.replace('\\', '/')

This does not actually edit the variable but instead returns a new variable as described here (https://docs.python.org/2/library/string.html#string.replace)

I suggest your code should be updated to file_path_on_target_machine = file_path_on_target_machine.replace('\\', '/')

This will remove the need for users to do this themselves before the saleae methods a file_path if using Windows

AndyEveritt commented 3 years ago

Although, for some of the methods it does not appear to be an issue whether the file path has \\ or / so it may be a null issue. If this is the case then I think the code should be removed since it is not needed or doing anything

ppannuto commented 3 years ago

Hah! Nice catch.. I don't actually have any Windows machines, so all of the windows code in this library is from other folks. Given that it's not actually doing anything and no one is complaining about that, probably best to just drop them. Happy to take a PR -- thanks!

AndyEveritt commented 3 years ago

I'm doing a project with the saleae currently so should have a good chance to confirm whether this is actually an issue or not. Either way I can certainly make a PR once I know