Skip to content

Bugfix/windows path #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 28, 2023
Merged

Bugfix/windows path #53

merged 12 commits into from
Jun 28, 2023

Conversation

murilopolese
Copy link
Contributor

Problem:

File management and navigation on Windows wasn't working because of different path separators and root.
The previous solution of using cleanPath on the front end to normalize the path strings.
The path strings is calculated using four state properties:

  • diskPath: The full path of the "working folder" on your disk
  • diskNavigation: Tells the current folder you are, relative to diskPath
  • serialPath: The serial port path
  • serialNavigation: Tells the current folder you are, relative to `serialPath

Solution:

Instead of storing the serial port path on serialPath, that is now stored on serialPort.
serialPath is now always / since you can't choose a folder inside of your board to be the "working folder"

The function cleanPath was replaced by 3 different methods in the Serial and Disk objects declared on preload.js.

The 3 methods are:

  • getFullPath(root, navigation, file): It's going to return the full resolved path
  • getNavigationPath(navigation, target): Safely concatenate the target to current navigation
  • getParentPath(filePath): Returns the parent path of a file or folder.

On Serial we can always assume UNIX-like filesystem so most of the methods will be basic string manipulation.
On Disk we use node's path module to comply to OS specification.

All ipcMain methods expect now a fully resolved file/folder path.

Extra:

Improve speed of downloading files without unsaved changes by simply saving the current editor content on disk.

Copy link
Collaborator

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the improvements in Windows VM and it works.
No regressions introduced to Mac

@ubidefeo
Copy link
Collaborator

fixes #50

Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes on Windows. Please go ahead with the merge, and let's revisit the path handling later.

@murilopolese murilopolese merged commit db8a85f into main Jun 28, 2023
@per1234 per1234 added the bug Something isn't working label Jun 28, 2023
@per1234 per1234 deleted the bugfix/windows-path branch June 28, 2023 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants