From 9965808f5b9bc1298b4269926e307024a4b57b5b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 1 Jan 2019 00:35:49 +0100 Subject: [PATCH] doxygen: improve #include information display based on real-world use. * In cases where Doxygen spots the namespace first in a *.cpp file (and later in a header) but the namespace is still contained in a single header, the include informations was improperly put locally into members, instead of globally. Now the information is propagated upwards from the members to ensure correctness. This needs also special handling for namespaces that are otherwise empty, where there's nothing that could propagate the info upwards. * Complex namespaces (namespaces containing other namespaces or classes) no longer have the include information global even though the non-class/namespace members are in the same file. This would be misleading since inner namespaces and classes are often in different files. --- doxygen/dox2html5.py | 65 +++++++++---- .../namespaceDeprecatedNamespace.html | 6 +- doxygen/test/compound_includes/First.h | 31 ++++++ .../namespaceContainsNamespace.html | 70 ++++++++++++++ ...aceContainsNamespace_1_1ContainsClass.html | 72 ++++++++++++++ .../compound_includes/namespaceEmpty.html | 33 +++++++ .../namespaceRoot_1_1Directory.html | 94 +++++++++++++++++-- doxygen/test/test_compound.py | 6 ++ 8 files changed, 350 insertions(+), 27 deletions(-) create mode 100644 doxygen/test/compound_includes/namespaceContainsNamespace.html create mode 100644 doxygen/test/compound_includes/namespaceContainsNamespace_1_1ContainsClass.html create mode 100644 doxygen/test/compound_includes/namespaceEmpty.html diff --git a/doxygen/dox2html5.py b/doxygen/dox2html5.py index 45c369a5..59e9a622 100755 --- a/doxygen/dox2html5.py +++ b/doxygen/dox2html5.py @@ -428,6 +428,11 @@ def parse_ref(state: State, element: ET.Element) -> str: return '{}'.format(url, class_, add_wbr(parse_inline_desc(state, element).strip())) +def make_include(state: State, file) -> Tuple[str, str]: + if file in state.includes and state.compounds[state.includes[file]].has_details: + return (html.escape('<{}>'.format(file)), state.compounds[state.includes[file]].url) + return None + def parse_id_and_include(state: State, element: ET.Element) -> Tuple[str, str, str, Tuple[str, str]]: # Returns URL base (usually saved to state.current_definition_url_base and # used by extract_id_hash() later), base URL (with file extension), and the @@ -440,14 +445,23 @@ def parse_id_and_include(state: State, element: ET.Element) -> Tuple[str, str, s include = None if state.current_kind in ['namespace', 'group']: file = element.find('location').attrib['file'] - if file in state.includes and state.compounds[state.includes[file]].has_details: - include = (html.escape('<{}>'.format(file)), state.compounds[state.includes[file]].url) - - # If the include differs from current compound include, reset it to signal - # that the compound doesn't have one unique include file. This will get - # later picked up by parse_xml() which either adds has_details to all - # compounds or wipes the compound-specific includes. - if state.current_include and state.current_include != file: + include = make_include(state, file) + + # If the include for current namespace is not yet set (empty string) + # but also not already signalled to be non-unique using None, set it to + # this value. Need to do it this way instead of using the location + # information from the compound, because namespace location is + # sometimes pointed to a *.cpp file, which Doxygen sees before *.h. + if not state.current_include and state.current_include is not None: + assert state.current_kind == 'namespace' + state.current_include = file + # parse_xml() fills compound.include from this later + + # If the include differs from current compound include, reset it to + # None to signal that the compound doesn't have one unique include + # file. This will get later picked up by parse_xml() which either adds + # has_details to all compounds or wipes the compound-specific includes. + elif state.current_include and state.current_include != file: state.current_include = None return id[:i], id[:i] + '.html', id[i+2:], include @@ -2347,21 +2361,36 @@ def parse_xml(state: State, xml: str): state.current_prefix = [] # Decide about the include file for this compound. Classes get it always, - # namespaces only if it we later don't discover that it's spread over - # multiple files; files and dirs don't need it (it's implicit) and it makes - # no sense for pages or modules. + # namespaces without any members too. state.current_kind = compound.kind - if compound.kind in ['struct', 'class', 'union', 'namespace']: - include = compounddef.find('location').attrib['file'] - if include in state.includes and state.compounds[state.includes[include]].has_details: - compound.include = (html.escape('<{}>'.format(include)), state.compounds[state.includes[include]].url) + if compound.kind in ['struct', 'class', 'union'] or (compound.kind == 'namespace' and compounddef.find('innerclass') is None and compounddef.find('innernamespace') is None and compounddef.find('sectiondef') is None): + file = compounddef.find('location').attrib['file'] + compound.include = make_include(state, file) # Save include for current compound. Every enum/var/function/... parser # checks against it and resets to None in case the include differs for # given entry, meaning all entries need to have their own include # definition instead. That's then finally reflected in has_details of # each entry. - state.current_include = include + state.current_include = file + + # Namespaces with members get a placeholder that gets filled from the + # contents; but only if the namespace doesn't contain subnamespaces or + # classes, in which case it would be misleading. It's done this way because + # Doxygen sets namespace location to the first file it sees, which can be + # a *.cpp file. In case the namespace doesn't have any members, it's set + # above like with classes. + # + # Once current_include is filled for the first time in + # parse_id_and_include(), every enum/var/function/... parser checks against + # it and resets to None in case the include differs for given entry, + # meaning all entries need to have their own include definition instead. + # That's then finally reflected in has_details of each entry. + elif compound.kind == 'namespace' and compounddef.find('innerclass') is None and compounddef.find('innernamespace') is None: + state.current_include = '' + + # Files and dirs don't need it (it's implicit); and it makes no sense for + # pages or modules. else: state.current_include = None @@ -2887,6 +2916,10 @@ def parse_xml(state: State, xml: str): # #include (for all others the include info is either on a compound itself # or nowhere at all) if state.doxyfile['SHOW_INCLUDE_FILES'] and compound.kind in ['namespace', 'group']: + # If we're in a namespace, its include info comes from inside + if compound.kind == 'namespace' and state.current_include: + compound.include = make_include(state, state.current_include) + # If we discovered that entries of this compound don't have a common # #include, flip on has_details of all entries and wipe the compound # include. Otherwise wipe the include information from everywhere but diff --git a/doxygen/test/compound_deprecated/namespaceDeprecatedNamespace.html b/doxygen/test/compound_deprecated/namespaceDeprecatedNamespace.html index ede92b56..b2bd4f1a 100644 --- a/doxygen/test/compound_deprecated/namespaceDeprecatedNamespace.html +++ b/doxygen/test/compound_deprecated/namespaceDeprecatedNamespace.html @@ -21,7 +21,6 @@

DeprecatedNamespace namespace -

A namespace.

@@ -108,6 +107,7 @@

enum DeprecatedNamespace::DeprecatedEnum +

An enum.

@@ -126,6 +126,7 @@

enum DeprecatedNamespace::Enum +

An enum.

@@ -146,6 +147,7 @@

typedef int DeprecatedNamespace::DeprecatedTypedef +

A typedef.

@@ -158,6 +160,7 @@ void DeprecatedNamespace::deprecatedFoo(int a, bool b, double c) +

A function.

@@ -168,6 +171,7 @@

int DeprecatedNamespace::DeprecatedVariable constexpr +

A variable.

diff --git a/doxygen/test/compound_includes/First.h b/doxygen/test/compound_includes/First.h index e43deb10..59945264 100644 --- a/doxygen/test/compound_includes/First.h +++ b/doxygen/test/compound_includes/First.h @@ -111,3 +111,34 @@ void foo(); #define A_DEFINE /*@}*/ + +/** +@brief This namespace should not have a global include + +Even though it's in a single file --- the contained namespace might or might +not be in the same file and that could be misleading. +*/ +namespace ContainsNamespace { + /** + * @brief This namespace should not have a global include either + * + * Because it has a class inside. + */ + namespace ContainsClass { + /** @brief Union */ + union Union {}; + + /** @brief This function should also have a local include */ + void foo(); + } + + /** @brief This function should have a local include */ + void foo(); +} + +/** +@brief This namespace should have a global include + +Even though it has no members that could set the global include for it. +*/ +namespace Empty {} diff --git a/doxygen/test/compound_includes/namespaceContainsNamespace.html b/doxygen/test/compound_includes/namespaceContainsNamespace.html new file mode 100644 index 00000000..bb0bf415 --- /dev/null +++ b/doxygen/test/compound_includes/namespaceContainsNamespace.html @@ -0,0 +1,70 @@ + + + + + ContainsNamespace namespace | My Project + + + + + +
+
+
+
+
+

+ ContainsNamespace namespace +

+

This namespace should not have a global include.

+
+

Contents

+ +
+

Even though it's in a single file — the contained namespace might or might not be in the same file and that could be misleading.

+
+

Namespaces

+
+
namespace ContainsClass
+
This namespace should not have a global include either.
+
+
+
+

Functions

+
+
+ void foo() +
+
This function should have a local include.
+
+
+
+

Function documentation

+
+

+ void ContainsNamespace::foo() +
#include <First.h>
+

+

This function should have a local include.

+
+
+
+
+
+
+ + diff --git a/doxygen/test/compound_includes/namespaceContainsNamespace_1_1ContainsClass.html b/doxygen/test/compound_includes/namespaceContainsNamespace_1_1ContainsClass.html new file mode 100644 index 00000000..46536430 --- /dev/null +++ b/doxygen/test/compound_includes/namespaceContainsNamespace_1_1ContainsClass.html @@ -0,0 +1,72 @@ + + + + + ContainsNamespace::ContainsClass namespace | My Project + + + + + +
+
+
+
+
+

+ ContainsNamespace::ContainsClass namespace +

+

This namespace should not have a global include either.

+
+

Contents

+ +
+

Because it has a class inside.

+
+

Classes

+
+
+ union Union +
+
Union.
+
+
+
+

Functions

+
+
+ void foo() +
+
This function should also have a local include.
+
+
+
+

Function documentation

+
+

+ void ContainsNamespace::ContainsClass::foo() +
#include <First.h>
+

+

This function should also have a local include.

+
+
+
+
+
+
+ + diff --git a/doxygen/test/compound_includes/namespaceEmpty.html b/doxygen/test/compound_includes/namespaceEmpty.html new file mode 100644 index 00000000..9fca57e3 --- /dev/null +++ b/doxygen/test/compound_includes/namespaceEmpty.html @@ -0,0 +1,33 @@ + + + + + Empty namespace | My Project + + + + + +
+
+
+
+
+

+ Empty namespace +
#include <First.h>
+

+

This namespace should have a global include.

+

Even though it has no members that could set the global include for it.

+
+
+
+
+ + diff --git a/doxygen/test/compound_listing/namespaceRoot_1_1Directory.html b/doxygen/test/compound_listing/namespaceRoot_1_1Directory.html index 7a3c42c7..1cb98471 100644 --- a/doxygen/test/compound_listing/namespaceRoot_1_1Directory.html +++ b/doxygen/test/compound_listing/namespaceRoot_1_1Directory.html @@ -37,7 +37,6 @@

Root::Directory namespace -

Namespace in directory.

@@ -85,7 +84,7 @@

Enums

- enum Enum { } + enum Enum { }
An enum.
@@ -94,11 +93,11 @@

Typedefs

- using Int = int + using Int = int
A typedef.
- using Float = float + using Float = float
An using declaration.
@@ -107,7 +106,7 @@

Functions

- void foo() + void foo()
A function.
@@ -116,7 +115,7 @@

Variables

- const int Var constexpr + const int Var constexpr
A variable.
@@ -125,23 +124,98 @@

A group

- enum Flag { } + enum Flag { }
Flag in a group.
- using Main = void + using Main = void
Alias in a group.
- void* variable constexpr + void* variable constexpr
Variable in a group.
- void bar() + void bar()
Function in a group.
+
+

Enum documentation

+
+

+ enum Root::Directory::Enum + +

+

An enum.

+
+
+

+ enum Root::Directory::Flag + +

+

Flag in a group.

+
+
+
+

Typedef documentation

+
+

+ typedef int Root::Directory::Int + +

+

A typedef.

+
+
+

+ using Root::Directory::Float = float + +

+

An using declaration.

+
+
+

+ using Root::Directory::Main = void + +

+

Alias in a group.

+
+
+
+

Function documentation

+
+

+ void Root::Directory::foo() + +

+

A function.

+
+
+

+ void Root::Directory::bar() + +

+

Function in a group.

+
+
+
+

Variable documentation

+
+

+ const int Root::Directory::Var constexpr + +

+

A variable.

+
+
+

+ void* Root::Directory::variable constexpr + +

+

Variable in a group.

+
+
diff --git a/doxygen/test/test_compound.py b/doxygen/test/test_compound.py index 6597ee38..334ec2d7 100644 --- a/doxygen/test/test_compound.py +++ b/doxygen/test/test_compound.py @@ -261,6 +261,12 @@ class Includes(IntegrationTestCase): self.assertEqual(*self.actual_expected_contents('classClass.html')) self.assertEqual(*self.actual_expected_contents('group__group.html')) + # These two should all have local includes because otherwise it gets + # misleading; the Empty namespace a global one + self.assertEqual(*self.actual_expected_contents('namespaceContainsNamespace.html')) + self.assertEqual(*self.actual_expected_contents('namespaceContainsNamespace_1_1ContainsClass.html')) + self.assertEqual(*self.actual_expected_contents('namespaceEmpty.html')) + class IncludesDisabled(IntegrationTestCase): def __init__(self, *args, **kwargs): super().__init__(__file__, 'includes_disabled', *args, **kwargs) -- 2.30.2