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

Namespace show all #336

Merged
merged 6 commits into from Jun 4, 2020
Merged

Namespace show all #336

merged 6 commits into from Jun 4, 2020

Conversation

amolenaar
Copy link
Member

@amolenaar amolenaar commented Jun 1, 2020

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes

What is the current behavior?

Issue Number: #334

What is the new behavior?

All namespaced information is shown.

Does this PR introduce a breaking change?

  • Yes
  • No

@amolenaar
Copy link
Member Author

The models shows all elements, which is something that needs fixing:

image

@danyeaw
Copy link
Member

danyeaw commented Jun 2, 2020

I really like this approach. I think autoformatting the name using the umlfmt module and removing the icon were nice touches.

A few things I have been thinking about:

  1. Should we have tools in the tool palette under the blocks section to create a Composition Relationship and Aggregation Relationship directly as a shortcut to creating an association, adding navigation, and setting shared or composite aggregation?
  2. Should we automatically name the Property of these types of relationships? For example, I know some of the other tools name the property name the same as the class name, except as lower camel case instead of upper camel case like the class name.

Right now if you create an unnamed association it just shows up as a blank row in the namespace:
image

@amolenaar
Copy link
Member Author

  1. Should we have tools in the tool palette under the blocks section to create a Composition Relationship and Aggregation Relationship directly as a shortcut to creating an association, adding navigation, and setting shared or composite aggregation?

This makes sense, I think, since now we need to head over to the editor to change the association ends.

  1. Should we automatically name the Property of these types of relationships? For example, I know some of the other tools name the property name the same as the class name, except as lower camel case instead of upper camel case like the class name.

For composite and aggregate associations it makes sense, for sure. Since there already is this directional aspect to it, with navigability.

Right now if you create an unnamed association it just shows up as a blank row in the namespace:

Hmm.. It should at least show something. Even <None> would be better than nothing.

@amolenaar amolenaar marked this pull request as draft June 2, 2020 09:06
@amolenaar
Copy link
Member Author

We still need to deal with relationships, too.

@amolenaar
Copy link
Member Author

I made attribute formatting a little more liberal, now it will render a string, even when there is no name for the property. Also it will parse associations with a : Type defined and just ignore that type (needed for editing in the tree view).

@amolenaar
Copy link
Member Author

Let's handle relationships in a different PR.

@amolenaar amolenaar marked this pull request as ready for review June 3, 2020 21:00
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

Looks great!

gaphor/UML/tests/test_umllex.py Outdated Show resolved Hide resolved
This field is editable in the tree view and cases for both attributes
and associations are handled correctly.
@amolenaar amolenaar merged commit 23f6219 into master Jun 4, 2020
@amolenaar amolenaar deleted the namespace-show-all branch June 4, 2020 20:08
@danyeaw danyeaw added the feature A new feature label Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants