DYN-4034 : open file from json content#12671
Conversation
src/DynamoCore/Models/DynamoModel.cs
Outdated
| this.LinterManager); | ||
|
|
||
| workspace.FileName = filePath; | ||
| workspace.FileName = String.IsNullOrEmpty(filePath) ? "" : filePath; |
There was a problem hiding this comment.
what affect does this have? Isn't workspace.FileName going to end up being null "" or filepath in all cases?
There was a problem hiding this comment.
I just wanted to avoid having workspace.FileName set to null and set it to empty string instead so that I can mimic the exact FIleName setting of a new Home unsaved dyn file.
src/DynamoCore/Models/DynamoModel.cs
Outdated
| out WorkspaceModel workspace) | ||
| { | ||
| CustomNodeManager.AddUninitializedCustomNodesInPath(Path.GetDirectoryName(filePath), IsTestMode); | ||
| if (!String.IsNullOrEmpty(filePath)) |
There was a problem hiding this comment.
I guess this is good to do, though unnecessary as ScanNodeHeadersInDirectory already checks the directory exists and logs something.
|
Looks good @BogdanZavu -
|
|
LGTM @BogdanZavu with test from Mike's comment |
| out WorkspaceModel workspace) | ||
| { | ||
| CustomNodeManager.AddUninitializedCustomNodesInPath(Path.GetDirectoryName(filePath), IsTestMode); | ||
| if (!string.IsNullOrEmpty(filePath)) |
There was a problem hiding this comment.
I added this check back because Path.GetDirectoryName throws when filePath is null.
| { | ||
| // Log file open action and the number of nodes in the opened workspace | ||
| Dynamo.Logging.Analytics.TrackFileOperationEvent( | ||
| dynamoModel.CurrentWorkspace.Name, |
There was a problem hiding this comment.
Should we use something else here since Name will be an empty string ?
There was a problem hiding this comment.
does this match the implementation/result for disk based file open?
There was a problem hiding this comment.
ah, I see, the other uses filepath - maybe something that notes this is in memory open.
|
@mjkkirschner @saintentropy I added a command as well to be more inline with the regular OpenFileCommand and to benefit from the AskForSave utility on the Dynamo side. Please take another look |
| case "OpenFileCommand": | ||
| command = OpenFileCommand.DeserializeCore(element); | ||
| break; | ||
| case "OpenFileFromJsonCommand": |
| errorMsgString = String.Format(Resources.MessageUnkownErrorOpeningFile, "Json file content"); | ||
| } | ||
| model.Logger.LogNotification("Dynamo", commandString, errorMsgString, e.ToString()); | ||
| System.Windows.MessageBox.Show(errorMsgString); |
There was a problem hiding this comment.
I think we want to use the new themed dialog box where possible - @QilongTang @RobertGlobant20 can you point to an example of this?
There was a problem hiding this comment.
| return PathHelper.IsValidPath(filePath); | ||
| } | ||
|
|
||
| private bool CanOpenFromJson(object parameters) |
There was a problem hiding this comment.
what about reusing this helper?
Dynamo/src/DynamoUtilities/PathHelper.cs
Line 136 in ad0dcca
There was a problem hiding this comment.
Please reuse the helper, I put a lot of thoughts into it.. Feel free to update as well
|
|
||
| internal static void LoadDebugModesStatusFromConfig(string configPath) | ||
| { | ||
| if (!File.Exists(configPath)) return; |
5 participants
Purpose
Allow a workspace to be created from json content.
In Generative Design, we often work with multiple variants of the same graph for which currently we save a file on disk.
We want to avoid this as it is more efficient t work with the file content directly.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner @saintentropy
FYIs