Skip to content
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

naive fix for #212408 #212461

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

razielanarki
Copy link

@razielanarki razielanarki commented May 10, 2024

A naive fix for #212408:

notes/toughts:

  • the check is naive in it's current state:
    it assumes the this.isWindows also applies to the file service that provides the dirstat (i haven't had the time to investigate how the services instantiated by the dialog are related in code)
    this IMO may require more work either in simpleFileDialog (like trying to tocuh the destination in a Promise, and returning that) or the fileService (extra flags)
    any comments, pointers, suggestions and help welcome! - this being my first PR to the codebase
  • thus, the condition on line 845 may need to be split into 'read-only' and 'locked' checks
  • also, the localization message with the key remoteFileDialog.validateReadonlyFolder should be renamed remoteFileDialog.validateLockedFolder for correctness, while possibly altering remoteFileDialog.validateReadonlyFolder or both to be more concise (if possible?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants