Add option hiddenCategories

[tobiasschweizer]

resolves #1407

This PR is not ready for review yet.

[tobiasschweizer]

@Gerrit0 I could already add the declarations as described in #1407 (comment)

However, as for the category check I struggle a bit.

I would expect some logic as follows:

• get the category of the class that is currently being processed
• if the current class's category is contained in hiddenCategories, use project.removeReflection

[tobiasschweizer]

If I understand correctly, there are three processing steps defined for the category plugin:

1. onBegin
2. onResolve
3. onEndResolve

So would this have to go into onEndResolve (when everything is done) like:

    private onEndResolve(context: Context) {
        const project = context.project;
        this.categorize(project);
        if (this.hiddenCategories) {
             const reflections: Reflection[] = [];
             // check if `this.hiddenCategories` contains categories from `project`
             // push those reflections on `reflections `
             ...
             
             // remove reflections belonging to hidden categories from the documentation
             reflections.forEach(reflection => project.removeReflection(reflection))
        }
    }

How could I write a test for the new option?

[Gerrit0]

You have the right idea - however, unfortunately due to the architecture of some of the other plugins (GroupPlugin is one of them), removing reflections after the onBegin method will break things... which I completely forgot about when I said this should be an easy issue. I haven't build a solution for this problem yet - to handle it properly the removeReflection method would need to emit an event so plugins that store reflection data can properly remove it... I think there is an issue open for this somewhere. This has been on the list for a while, I just haven't gotten around to it since I haven't needed it, and doing it properly requires reviewing all the existing plugins to ensure they handle the event properly... I'm looking into how difficult this is now, I might be able to finally do it this week.

Adding a test for this is pretty easy at least, I think this option can always be set, so we don't need a new spec file type:

• In scripts/rebuild_specs.js and src/test/converter.test.ts add a hidden category in the options passed to app.bootstrap
• In src/test/converter/comment add a new file exporting some values - some with the category tag, some without.
• Run npm run rebuild_specs converter comment to generate a specs.json file with your changes

[tobiasschweizer]

@Gerrit0 I understand :-) So ping me once you have implemented the necessary functionality for removeReflection and then I will try to finish this PR.

[tobiasschweizer]

@Gerrit0 just a shot in the dark:
What about trying not to add the reflections that should be hidden/excluded? So we would not have to remove them.

[Gerrit0]

That could work as well - however that means that the categorization logic would have to happen earlier - instead of in the resolve event.... I'm not completely against that.

[Gerrit0]

I spent a few hours today trying to get the reflection removed event to work - this is trickier than I anticipated... TypeDoc is really bad about storing data in multiple places, and it is really easy to miss one of them... I think I mostly have it working except for inheritance. (@hidden comment on Parent in class Child extends Parent should totally hide parent... but it is only half being hidden)

[tobiasschweizer]

@Gerrit0 sorry that I cannot help you there as I am not familiar with the architecture. Which branch are you working on?

[Gerrit0]

I never ended up pushing any of the changes for the removal event. There are more architectural difficulties than I realized.

1. If removal can happen after the resolve step, every plugin which sets properties on reflections that reference another reflection needs to keep track of what properties are set so if that referenced reflection is removed, the reference can be removed.
2. Allowing removal during the resolve step is especially problematic since the order that reflections are resolved in can affect the resulting output.

I think I'll need to dedicate a couple solid days to really going through and understanding all of the relations in order to properly handle this. I think the "right" solution is to do removal after resolution, but I also really dislike that solution since it adds a ridiculous amount of property tracking... which would be really easy to mess up. This probably won't happen this year, so unfortunately I think the best bet for this feature is to do filtering in onBeginResolve like the comment plugin does.

#1177 has some more notes on what "removal" ought to mean.

[tobiasschweizer]

@lastthyme @Gerrit0 sorry, but this PR is not yet ready for review.

[tobiasschweizer]

@Gerrit0 Hi, any chance to revive this PR? ;-)

[Gerrit0]

This probably won't happen this year, so unfortunately I think the best bet for this feature is to do filtering in onBeginResolve like the comment plugin does. - Me 2 years ago

This probably won't happen ever. It massively complicates a ton of logic. If updated to do as described, I'm still open to this.

[tobiasschweizer]

This probably won't happen this year, so unfortunately I think the best bet for this feature is to do filtering in onBeginResolve like the comment plugin does. - Me 2 years ago

This probably won't happen ever. It massively complicates a ton of logic. If updated to do as described, I'm still open to this.

Shall I close this PR since I won't be able to work on this?

[Gerrit0]

It's still possible to do this - it just needs to be done at an earlier state. Removal needs to happen at EVENT_BEGIN_RESOLVE instead of later

[tobiasschweizer]

cool :-)