DYN-8100: Handle opening file paths that include archived locations #15851
Conversation
github-actions
bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8100
| var jsonDynFile = ResourceUtilities.LoadContentFromResources(GuidesManager.OnboardingGuideWorkspaceEmbeededResource, Assembly.GetExecutingAssembly(), false, false); | ||
| OpenFromJson(new Tuple<string, bool>(jsonDynFile, true)); | ||
| } | ||
|
|
||
| private bool CanOpen(object parameters) | ||
| { | ||
| var filePath = parameters as string; | ||
|
|
||
| if (filePath.Contains(".zip\\") || filePath.Contains(".ZIP\\")) |
There was a problem hiding this comment.
@LongNguyenP I think we need something more concrete as in Windows a directory can also be named with .zip in its name like, ABCD.zip. And in that case this check will cause problems. May be incrementally check if each part of the path is a directory except the last one?
There was a problem hiding this comment.
@zeuso
Yes I thought of that scenario, that was why the comparision pattern is ".zip\\" rather than just ".zip" . I think this is likely sufficient (without incremental checking each part of the path).
There was a problem hiding this comment.
what about 7zip archives? (.7z)
LongNguyenP
changed the title
| @@ -922,8 +922,8 @@ internal void CrashGracefully(Exception ex, bool fatal = false) | |||
| { | |||
| exceptionAssembly = ex.InnerException?.TargetSite?.Module?.Assembly; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
do you know why there are too many empty space changes?
Seems like there are too much changes but most of them are due to the space space removal.
There was a problem hiding this comment.
Yeah, i noticed those, no idea why they got added, but seems to be correct though, so I did not remove.
There was a problem hiding this comment.
In my experience if your editor isn't set to strip trailing spaces these creep in without anyone being the wiser. Might make sense to align on whether we make this setting? On Vera we had a linter which would fail on trailing space. That's going a bit far I think, but it did ensure this sort of thing was kept clean.
There was a problem hiding this comment.
Can you share the link to that linter? Was that at a PR check level?
There was a problem hiding this comment.
Well, the one I refer to was on TypeScript (tslint). But it looks like this could be accomplished with git hooks: https://stackoverflow.com/questions/591923/make-git-automatically-remove-trailing-white-space-before-committing.
There was a problem hiding this comment.
We have https://github.com/DynamoDS/Dynamo/blob/master/.editorconfig at repo root. We can add all the common rules there.
https://learn.microsoft.com/en-us/visualstudio/ide/create-portable-custom-editor-options
zeusongit
changed the title
| @@ -94,7 +94,7 @@ public DialogResult ShowDialog() | |||
|
|
|||
| IShellItem dialogResult; | |||
| _dialog.GetResult(out dialogResult); | |||
| dialogResult.GetDisplayName(SIGDN.SIGDN_FILESYSPATH, out _fileName); | |||
| dialogResult.GetDisplayName(SIGDN.SIGDN_DESKTOPABSOLUTEEDITING, out _fileName); | |||
There was a problem hiding this comment.
Is this because the SIGDN_FILESYSPATH is not shown for some cases?
There was a problem hiding this comment.
Yes, in cases where zip file was part of the path, it threw an error.
Purpose
The PR handles opening file paths that include archived locations.
In Windows, it is possible to explore a zip archive without extracting it, the PR addresses those cases where the file path includes a zipped(archived) location for example:
C:\Program Files\Archive.zip\Graph.dynin this case an error message will be displayed.Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Handle opening invalid file paths such as archives, and show an error message
Reviewers
@DynamoDS/dynamo