[RFC] Beautify declaration navigation context by parsing doxygen commands
AbandonedPublic

Authored by progwolff on Apr 15 2018, 11:00 AM.

Details

Reviewers
None
Group Reviewers
KDevelop
Summary

With this patch, the following doxygen commands in comments are replaced by html:

  • \param, \param[in], ...
  • \return, \returns
  • \brief
  • \see, \since, \note, \todo, \bug
  • \a, \e, \em, \c, \\p
  • Code blocks and formulas: \code, \endcode, \f$, \f[, \f{...}{
  • Escape sequences: \$, \@, \\, \&, \~, \<, \>, \#, \%, \", \., \n, \|, \::, \--, \---
Test Plan

Diff Detail

Repository
R32 KDevelop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
progwolff created this revision.Apr 15 2018, 11:00 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptApr 15 2018, 11:00 AM
progwolff requested review of this revision.Apr 15 2018, 11:00 AM
progwolff edited the test plan for this revision. (Show Details)Apr 15 2018, 11:01 AM
progwolff updated this revision to Diff 32172.Apr 15 2018, 11:02 AM
  • remove newline
progwolff edited the summary of this revision. (Show Details)Apr 15 2018, 11:03 AM
croick added a subscriber: croick.Apr 15 2018, 11:19 AM

This is great! Could you add \ref, \link and \endlink as well?

croick added inline comments.Apr 15 2018, 11:27 AM
kdevplatform/language/duchain/navigation/abstractdeclarationnavigationcontext.cpp
803

That should be <strong>\\2</strong> or so (including the appropriate regexp), right?

progwolff added a comment.EditedApr 15 2018, 11:39 AM

This is great! Could you add \ref, \link and \endlink as well?

I am not sure how we could generate appropriate link targets.
We only have the comment's text and don't know anything about the project and its objects (files, classes, functions, ...) or do we?

progwolff added inline comments.Apr 15 2018, 11:48 AM
kdevplatform/language/duchain/navigation/abstractdeclarationnavigationcontext.cpp
803

Sorry, missed that one.

How about <strong>See also:</strong><br />?

progwolff updated this revision to Diff 32177.Apr 15 2018, 11:48 AM
  • fix @see
progwolff updated this revision to Diff 32179.Apr 15 2018, 12:07 PM
  • fix newline escape sequence
mwolff added a subscriber: mwolff.Apr 16 2018, 9:01 AM

I'm very unsure about this. On one hand, this obviously looks better in your screenshots. On the other hand, what if we encounter comments that are formatted with some other syntax? Most notably consider how your code would lead to broken HTML quite easily, e.g. when only part of a match is encountered (such as only \f\[ but no \f\]).

Furthermore, you are introducing untranslatable strings that may be odd when rendered (e.g. when the comment is actually German or such). If at all, then I would say we should keep the doxygen strings and just apply formatting, but not change any string conents. So @see will be rendered as @see, instead of See also:

kdevplatform/language/duchain/navigation/abstractdeclarationnavigationcontext.cpp
792
for (auto lineRef : comment.splitRef(QLatin1Char('\n')) {
    QString line = lineRef.trimmed();
    ...
}
795

please don't use QRegExp in new code, use QRegularExpression instead

also, wrap all string literals in QStringLiteral

I'm very unsure about this. On one hand, this obviously looks better in your screenshots. On the other hand, what if we encounter comments that are formatted with some other syntax?

Do you have examples for comments in a different syntax? I only know javadoc, which is quite similar.
Maybe there is a way to add the proposed functionality to the clang backend instead. This way we could be sure that the comments we parse are always doxygen comments.

Most notably consider how your code would lead to broken HTML quite easily, e.g. when only part of a match is encountered (such as only \f\[ but no \f\]).

I might fix this by applying the Regex on the whole comment instead of single lines.

Furthermore, you are introducing untranslatable strings that may be odd when rendered (e.g. when the comment is actually German or such).

In german language conversations between colleagues I usually hear phrases like "Die Funktion return't einen integer" more often than "Die Funktion gibt eine Ganzzahl zurück". I see your point, but I don't consider it that critical. Most programming languages have english keywords anyway.

If at all, then I would say we should keep the doxygen strings and just apply formatting, but not change any string conents. So @see will be rendered as @see, instead of See also:

How about making this feature optional with the states "don't parse doxygen comments", "parse doxygen comments, but keep commands", "parse doxygen comments replacing commands"?

There's qdoc, doxypress, proprietary custom formatting styles... Comments are free form after all, anything might be in there.

Re parsing whole comment instead of lines: That's a good idea, but you probably don't want to parse using regular expressions, since this can still be error prone (think about code blocks - you don't want to touch their contents).

I personally dislike having a separate setting for this. Overall, imo, I really think we should instead fix clang upstream so that we can re-enable this feature as we got it through libclang. See also:

https://phabricator.kde.org/D8857
https://bugs.llvm.org/show_bug.cgi?id=35333

progwolff abandoned this revision.Apr 16 2018, 12:58 PM

Overall, imo, I really think we should instead fix clang upstream so that we can re-enable this feature as we got it through libclang. See also:

https://phabricator.kde.org/D8857
https://bugs.llvm.org/show_bug.cgi?id=35333

I didn't know that libclang has such a feature. I'll take a look at this when I find some more time.