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

Short namespace heuristics for Autocomplete #4060

Merged
merged 6 commits into from
Mar 25, 2015

Conversation

aparajit-pratap
Copy link
Contributor

When there are namespace collisions, i.e. the same class name belonging to more than one namespace that are imported into Dynamo (via Import Library, Package manager, etc.), auto-complete would display the conflicting classes with their fully qualified names in order to distinguish them uniquely. For example:

Autodesk.DS.Geometry.Point
Rhino.Geometry.Point
FFITarget.Dynamo.Point

This submission enhances this experience by displaying the shortest possible names, such that they can still be resolved uniquely. So for the same example above, auto-complete now displays:

Autodesk.Point
Rhino.Point
FFITarget.Point

The core algorithm basically computes all possible in-order permutations of the individual namespaces in the long name and checks for which of these permutations, starting with the shortest, resolves uniquely. These shortest names are then fed into auto-complete to display instead of the fully qualified names used earlier.

Note that in the case of an existing resolution map in the namespace resolver, where there could be existing mappings to map say Point to Autodesk.DS.Geometry.Point, auto-complete will refer to the resolver as well. Thus in the same example above, it will display:

Point
Rhino.Point
FFITarget.Point

Unfortunately in the case of namespace collisions, users will need to get in the habit of seeing at least one namespace preceding the class name. We cannot get rid of them entirely.

Reviewers:
@Benglin
@junmendoza

@aparajit-pratap aparajit-pratap changed the title DNM: Short namespace heuristics Short namespace heuristics for Autocomplete Mar 23, 2015
@Benglin
Copy link
Contributor

Benglin commented Mar 24, 2015

Hi @junmendoza please have a look at this, and I'll be back to see it later. Thanks :)

@aparajit-pratap
Copy link
Contributor Author

Since most developers are busy or out of action, @sharadkjaiswal please give this a look. Thanks.

/// <param name="thisSymbol"></param>
/// <param name="otherSymbol"></param>
/// <returns></returns>
public static bool operator ==(Symbol thisSymbol, Symbol otherSymbol)
Copy link
Contributor

Choose a reason for hiding this comment

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

was there a specific reason to use an overload op as opposed to a normal function name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well that's a C# thing (best practice) to overload the equality operator if you're overriding object.Equals so that both methods have the same behavior for the given type so as not to confuse users of that type.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's the other way, when you are overloading operator ==, you must override object.Equals. If you have overridden object.Equals then even if you don't overload operator ==, it should call object.Equals internally.
https://msdn.microsoft.com/en-us/library/7h9bszxx(v=vs.85).aspx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No equality operator does not know about object.Equals as it is purely a C# thing and has not got anything to do with .NET, which is what defines object.Equals. Equality operator does not use object.Equals internally.

@junmendoza
Copy link
Contributor

Unless my comments reveal somthing glaring... LGTM

@aparajit-pratap
Copy link
Contributor Author

Thanks @junmendoza!

aparajit-pratap added a commit that referenced this pull request Mar 25, 2015
Short namespace heuristics for Autocomplete
@aparajit-pratap aparajit-pratap merged commit 657b59b into DynamoDS:master Mar 25, 2015
@aparajit-pratap aparajit-pratap deleted the shortNames branch March 25, 2015 07:04
@Benglin
Copy link
Contributor

Benglin commented Mar 26, 2015

Thanks for helping to review, @junmendoza!

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