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

Wrong QHP index structure #9094

Closed
ferdymercury opened this issue Feb 4, 2022 · 11 comments
Closed

Wrong QHP index structure #9094

ferdymercury opened this issue Feb 4, 2022 · 11 comments
Labels

Comments

@ferdymercury
Copy link
Contributor

ferdymercury commented Feb 4, 2022

Describe the bug
QHP generation is currently incorrect in master. The tree structure of classes is recursively nested, instead of all at the same level.
index.qhp.zip

Expected behavior
QHP index is similar to the HTML index.

Screenshots
If i go before commit 2952581 i.e. #9069, the QHP result is
image

The HTML result is correct:
image

To Reproduce

  • Download root-docu.zip
  • cd documentation/doxygen
  • doxygen -d qhp
  • Check html/index.qhp

Version
1.9.4 (f8bf952*)

Stack trace

Error in line 30: Opening and ending tag mismatch.
error: failed to run qhelpgenerator on index.qhp

Additional context
Part of this has been discussed in the "space stripping" issue: #9058
but to keep the discussion clean, I open a new issue.

@ferdymercury
Copy link
Contributor Author

ferdymercury commented Feb 4, 2022

With the following diff,

diff --git a/src/index.cpp b/src/index.cpp
index 24a62823..d23948b5 100644
--- a/src/index.cpp
+++ b/src/index.cpp
@@ -331,7 +331,7 @@ static void writeMemberToIndex(const Definition *def,const MemberDef *md,bool ad
       Doxygen::indexList->decContentsDepth();
     }
   }
-  Doxygen::indexList->closeContentsItem();
+  //Doxygen::indexList->closeContentsItem();
 }
 
 //----------------------------------------------------------------------
@@ -418,7 +418,7 @@ void addMembersToIndex(T *def,LayoutDocManager::LayoutPart part,
         }
       }
     }
-
+    Doxygen::indexList->closeContentsItem();
     Doxygen::indexList->decContentsDepth();
   }
 }

it looks almost correct:

image

but "Tutorials" appears one too much to the left. Maybe because it's empty or because tab type is "user"?

@doxygen
Copy link
Owner

doxygen commented Feb 20, 2022

I decided to reimplement the Qhp indexer completely, see referenced commit. I also removed the closeContentsItem() method in the process. Let me know if it works.

@ferdymercury
Copy link
Contributor Author

ferdymercury commented Feb 20, 2022

Thanks so much. I just recompiled it.

Some observations:

I had to add this for it to compile:

diff --git a/src/qhp.cpp b/src/qhp.cpp
index 64c4e1cb..89292f3e 100644
--- a/src/qhp.cpp
+++ b/src/qhp.cpp
@@ -16,7 +16,7 @@
 #include <memory>
 #include <string.h>
 #include <vector>
-
+#include <cassert>
 
 #include "config.h"
 #include "debug.h"

Qhelpgenerator(qt4) stops with the following error:
Error in line 953: Unexpected '<'.

Here the problematic qhp part:

<section title="operator!=" ref="classTDatime.html#a931774b812dd58fae53a70f3f8c860fc"/>
<section title="operator<" ref="classTDatime.html#ae22701a32ab638813aa95dae18e54074"/>
<section title="operator<=" ref="classTDatime.html#a18aeb579552807abe3a03105ced3093e"/>
<section title="operator==" ref="classTDatime.html#af18500b22a926d85e8a45be9a3746254"/>

Here the qhp file:
index.qhp.zip

I think these characters need to be escaped: https://stackoverflow.com/a/1091953/7471760

Here the qhp file after replacing by hand the "<" with &lt;. I didn't need to replace anything else. The > seems to be fine.

After running qhelpgenerator on that one, I get the proper tree!! Thanks so much!

image

However, there seems to be a problem with the paths. When I open the file with qtassistant, the following warning is shown:
QTextBrowser: No document for qthelp://cern.ch.root/doc/index.html

It might be though a problem on where qthelpgenerator is being run or something like that.

doxygen added a commit that referenced this issue Feb 20, 2022
@doxygen
Copy link
Owner

doxygen commented Feb 20, 2022

@ferdymercury I fixed the compilation issue and added escaping of the special XML characters. Not sure yet about the path problem. If you know what needs to be changed, then please let me know.

@ferdymercury
Copy link
Contributor Author

ferdymercury commented Feb 20, 2022

Thanks! I can confirm that the escaped characters and compile issues are now fixed. :)

One minor issue. It seems that in the resulting qhp file, there are some duplicated files listed:

      <file>functions.html</file>
      <file>functions.html</file>
      <file>functions.html</file>
      <file>functions.html</file>
      <file>functions.html</file>
      <file>functions_func.html</file>
      <file>functions_func.html</file>
      <file>functions_func.html</file>
      <file>functions_func.html</file>
      <file>functions_func.html</file>
      <file>files.html</file>
      <file>globals.html</file>
      <file>globals.html</file>
      <file>globals.html</file>
      <file>globals_func.html</file>
      <file>globals_func.html</file>
      <file>globals_func.html</file>

--> Should I open a separate issue for this ?

Just as a side note, I noticed that the qhp problems had been also observed in 2014 within LLVM. https://reviews.llvm.org/D2967

Concerning the path issue, I just found out that it's an issue with qt4-assistant. The issue disappears with qt5-assistant. Thanks!

@albert-github
Copy link
Collaborator

The problem regarding the double entries in the files part can also be reproduced for the doxygen internal documentation for (with the number of times this occurs):

      2 <keyword name="FortranFormat_Fixed" id="types.h::FortranFormat_Fixed" ref="d9/d49/types_8h.html#ad3f2a8c13ceee9c0aaeabf930dd88266aeb38ac735dad655f0d49d92a9e4b68e9"/>
     12 <file>namespacemembers_func.html</file>
      3 <file>namespacemembers_eval.html</file>
     14 <file>functions_type.html</file>
     19 <file>functions_enum.html</file>
     10 <file>functions_rela.html</file>
     19 <file>globals_type.html</file>
     16 <file>globals_enum.html</file>

as this is not a problem regarding the structure, best is to create a new issue.

@ferdymercury
Copy link
Contributor Author

Ok, thanks, will do. Shall I close this issue then ?

@albert-github albert-github added the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Feb 21, 2022
@albert-github
Copy link
Collaborator

Code has been corrected in master on GitHub (please don't close the issue as this will be done at the moment of an official release).

@albert-github
Copy link
Collaborator

Some remarks:

  • in the old implementation with the -d qhp not only the leading spaces were filtered but also the \n.
  • I don't think it is a good idea to have on multiple places index.qhp defined (doxygen.cpp and qhp.cpp), probably better as a constant in qhp.h or as a static function in the Qhp class
  • The getQchFileName is (and was) defined as a static function in doxygen.cpp, I think it might be better to define is as static function in the Qhp class.

@ferdymercury
Copy link
Contributor Author

Some remarks:

* in the old implementation with the `-d qhp` not only the leading spaces were filtered but also the `\n`.

From the point of view of usability, I think that removing also the '\n' as was done in the earlier implementation was too much of an optimization, because if you try to open this file with 'nano' or any text editor, it gets stuck forever if your qhp is 300Mb long, as it is all in one line, and at least my CPU was smoking hot. So stripping the spaces but not the \n, as done now, is a good compromise for visibility, as there are 99x more spaces than '\n'. (So I like it more as it is now).

Of course, before one could still go to -d qhp, but then in some cases the file might become very big, so "release" mode is now just great for a quick glance, and then -d for debugging of section alignment, etc.

@doxygen
Copy link
Owner

doxygen commented May 5, 2022

This issue was previously marked 'fixed but not released',
which means it should be fixed in doxygen version 1.9.4.
Please verify if this is indeed the case. Reopen the
issue if you think it is not fixed and please include any additional information
that you think can be relevant (preferably in the form of a self-contained example).

@doxygen doxygen removed the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label May 5, 2022
@doxygen doxygen closed this as completed May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants