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

use full class name as ID of a tag #242

Merged
merged 1 commit into from
Dec 17, 2016

Conversation

ahus1
Copy link
Contributor

@ahus1 ahus1 commented Oct 9, 2016

In our project we use very many tags, and we order them in hierarchies and with inner classes. Sometimes inner classes have the same names (as they are the leaves of our tree). This is currently not supported by JGiven, as it uses the class' simpleName to index the tags. I know that this is to some extent documented in @istag#name - but it seems to be a bit outdated as it talks about "type name".

This tries to introduce the full class name to derive the IDs of the tags, while preserving the existing functionality to use the simple name as the default name of the tag. The ID will only be used for the internal indexing, and will not be shown to the user.

I've tested this with the HTML5 report and it works for me AFAICS. Let me know what you think - I wonder if it breaks functionality or (too much) backwards compatibility.

In addition to this the ...#/tag/name navigation used in the HTML5 report should use something like ...#/tagid/id to uniquely identify tags not by their name, but by their ID. I'll look into this once we agree on the Java code.

@janschaefer
Copy link
Contributor

I haven't tested yet, but I wonder how this works without actually adapting the HTML report :-)

@janschaefer
Copy link
Contributor

I think the change is good and I also agree with your suggestion of introducing a tagid/id navigation

@ahus1
Copy link
Contributor Author

ahus1 commented Dec 17, 2016

The javascript changes have finally arrived for this PR. the /tagid URL fragment is now used when selecting a tag from the navigation on the left (as long as it is not a name based tag), or when clicking on a tag next to a test case.

@janschaefer
Copy link
Contributor

Looks good. Thank you!

@janschaefer janschaefer merged commit f2785c9 into TNG:master Dec 17, 2016
janschaefer pushed a commit that referenced this pull request Dec 17, 2016
@ahus1 ahus1 deleted the ahus1_tag_id_class_name branch December 31, 2016 17:09
@janschaefer janschaefer added this to the v0.14.0 milestone Jan 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants