Skip to content

Conversation

jose-ba
Copy link
Contributor

@jose-ba jose-ba commented Dec 3, 2024

See this issue: #89

@smeghead
Copy link
Owner

smeghead commented Dec 4, 2024

@jose-ba

GREAT WORK!!! Thank you very much.

I am starting to review the updates.

It looked like I was only filtering relations, but the magic of PlantUML remove @unlinked hides unrelated classes so that I can draw the desired diagram. I did not know about remove @unlinked.

I have tried to output a figure with a simple example.

Normal Diagram

rel-default

--from=Entry

rel-filter-from

--to=Entry

rel-filter-to

I have a concern that the simple words from and to might be a bit too generic for the option to add. I would prefer a name that is easily recognizable as the target of the filtering. I will consider a few candidates.

@jose-ba
Copy link
Contributor Author

jose-ba commented Dec 4, 2024

Thank you!

I agree, the names aren't great. Let me know if you can think of any better ones and I'll be happy to make the change.

@smeghead
Copy link
Owner

smeghead commented Dec 5, 2024

@jose-ba

This feature filters the classes to be displayed by the associations the target class has.

I suggest the option names --rel-target-from and --rel-target-to.

--rel-target-from: Include in the target the classes on which the target class depends.
--rel-target-to: Include classes that depend on the target class.

I would also like to add an option to include both directions of dependencies.
--rel-target: Include classes on which the target class depends as well as classes that depend on the target class.

In situations where we don't have much knowledge about the classes we want to investigate, we believe that --rel-target will give us a rough idea of how they are related.

I would suggest that --depth also be changed to --rel-target-depth to match the above change to make it more explicit that it is a relevant option.

What do you think?

both direction(--rel-target=Entry)

rel-filter-both

@smeghead smeghead added the enhancement New feature or request label Dec 5, 2024
@jose-ba
Copy link
Contributor Author

jose-ba commented Dec 5, 2024

I was thinking about --from-class and --to-class but I think that I like your idea more because they have a common prefix (so it is clear that they do something similar and are related to --rel-target-depth) and is more explicit. I will make the change tomorrow if I can. Changing the names is easy obviously, but I have to implement and test the --rel-target option, I'm almost sure that I can make it work just by setting --rel-target-from and --rel-target-to to the same value internally, but I want to test it.

Thank you

@jose-ba
Copy link
Contributor Author

jose-ba commented Dec 6, 2024

I have added the --rel-target option and renamed the other ones. Please review the new option when you can.

Copy link
Owner

@smeghead smeghead left a comment

Choose a reason for hiding this comment

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

Thank you!
Please check the two points noted.

Copy link
Owner

@smeghead smeghead left a comment

Choose a reason for hiding this comment

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

@jose-ba
Thank you very much!!!

@smeghead smeghead merged commit 1c33b21 into smeghead:main Dec 7, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants