Dyn 4715 package name special characters#12802
Dyn 4715 package name special characters#12802QilongTang merged 29 commits intoDynamoDS:masterfrom jesusalvino:DYN-4715-Package-Name-Special-Characters
Conversation
Update from the main repository
| </Setter> | ||
| </Style> | ||
|
|
||
| <Style x:Key="InputStyleWithIcon" TargetType="{x:Type TextBox}"> |
There was a problem hiding this comment.
What are the changes to this style?
There was a problem hiding this comment.
What are the changes to this style?
adds an extra space to the right side of the input to draw the Icon that will be displayed if required
src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs
Outdated
Show resolved
Hide resolved
|
Regression also to look at |
|
One comment then LGTM |
|
So just to be clear this will not affect the packages which already have spaces and want to publish a new version, but will prohibit future package names to include spaces, right? @QilongTang @jesusalvino |
What about users publishing new versions of existing packages with spaces? Won't this stop them from updating existing packages? - same question as @zeusongit 👍 |
| { | ||
| return element.Contains("*") || element.Contains(".") || element.Contains(" ") | ||
| || element.Contains("\\"); | ||
| // Excluding white spaces and uncommon characters, only keeping the displayed in the Windows alert |
There was a problem hiding this comment.
FYI.. @zeusongit @mjkkirschner see this comment, space is excluded from .NET API so not considered special char in Dynamo
See my comment in code |
|
@jesusalvino Looks like there are still regressions to deal with before we can merge |
| { | ||
| if (invalidCharacters == null) | ||
| { | ||
| invalidCharacters = Search.SearchDictionary<object>.SpecialAndInvalidCharacters(); |
There was a problem hiding this comment.
Looking at this PR again, I still would not prefer this way. The original getter of invalid chars lives inside of search dictionary which seems strange. Can you move this static getter to https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoUtilities/PathHelper.cs and have ContainsSpecialCharacters refer to this property? And instead of the getter pointing to a function elsewhere, I would just return System.IO.Path.GetInvalidFileNameChars().Where(x => !char.IsWhiteSpace(x) && (int)x > 31).ToArray();
QilongTang
left a comment
There was a problem hiding this comment.
More changes requested @jesusalvino

Purpose
Prevent user from enter some special characters as part of the package name in the Publish Package feature.
Declarations
Check these if you believe they are true
*.resxfiles###Reviewers
@QilongTang
###FYIs
@RobertGlobant20 @filipeotero