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

Data storage (i.e. extension and registered root path) for version control extensions inside Dev Home #2984

Conversation

ssparach
Copy link
Contributor

@ssparach ssparach commented May 22, 2024

Summary of the pull request

This PR contains the changes required to store and retrieve data from a json file required for source control integration.

References and relevant issues

Detailed description of the pull request / Additional comments

The code changes included are:

  • Read/Write extension, root path pairs (JSON file)
  • Temporary UI (the UI currently outlined in this PR will be removed and is in place currently to provide basic add functionality). The new UI will be in accordance to design specifications and is being tracked.
  • Unit Test

Validation steps performed

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

if (!Path.IsPathFullyQualified(rootPath))
{
// Error: Path is not fully qualified or maybe a UNC path.
RootPathErrorBar.IsOpen = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Change the error message based on what is being checked. Give the user the exact reason why the path isn't valid.

Copy link
Member

Choose a reason for hiding this comment

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

NIT: Change the error message based on what is being checked. Give the user the exact reason why the path isn't valid.

+1 to this. We may even consider having some different error/exception types for path checking.

Once interesting example is that repositories mounted on WSL look sorta like UNC paths, but we should try to detect that case separate from UNC paths. Also with WSL repos, their file ownership looks off from Windows' point of view, so Git will raise specific errors about those cases. Rather than outright failing, we should offer a suggestion to the user to help them enable Git to work with those WSL repos, aka git config --global --add safe.directory <path to repo>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great! Currently, the UI in place is temporary. I will incorporate this feedback and gain more clarity around the design for surfacing user facing error messages in the expected UX.

@ssparach ssparach merged commit 32e55b0 into feature/fileexplorer-sourcecontrol-integration Jun 3, 2024
1 of 4 checks passed
@krschau krschau added this to the Dev Home 0.15 milestone Jun 3, 2024
ssparach added a commit that referenced this pull request Jul 31, 2024
…ntrol extensions inside Dev Home (#2984)

* changes to write/read data (extension information + root path) from json

* address PR comments

* revert line endings

* address PR comments
ssparach added a commit that referenced this pull request Jul 31, 2024
…ntrol extensions inside Dev Home (#2984)

* changes to write/read data (extension information + root path) from json

* address PR comments

* revert line endings

* address PR comments
ssparach added a commit that referenced this pull request Jul 31, 2024
…ntrol extensions inside Dev Home (#2984)

* changes to write/read data (extension information + root path) from json

* address PR comments

* revert line endings

* address PR comments
@ssparach ssparach deleted the user/ssparach/datastorage branch August 2, 2024 22:34
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.

4 participants