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

DOXYGEN: Doxygen improvements part 3 #2499

Open
wants to merge 1 commit into
base: master
from

Conversation

@b-gent
Copy link
Contributor

@b-gent b-gent commented Oct 7, 2020

Editing and adding doxygen comments in a series of header files (see diff).

Plus some small changes in the config file.

HTML output is uploaded here -> https://b-gent.github.io/PR2499/d3/d79/group__common__dialogs.html

@b-gent b-gent changed the title DOXYGEN: Doxygen improvements part 3 [DNM] DOXYGEN: Doxygen improvements part 3 Oct 7, 2020
@b-gent
Copy link
Contributor Author

@b-gent b-gent commented Oct 7, 2020

Please do not merge yet.


// if the unaligned load and the byteswap take alot instructions its better to directly read and invert
/** @name Funtions for directly reading/writing and inverting

This comment has been minimized.

@mgerhardy

mgerhardy Oct 8, 2020
Contributor

There is a typo here. It also looks like two closing doxygen section braces in this file

This comment has been minimized.

@b-gent

b-gent Oct 8, 2020
Author Contributor

Right, the typo was here originally and I didn't catch it, thanks.

There are two closing tags at the end of the file because the first one closes this particular group (if it should be closed earlier, let me know), and the second one closes the main doxygen group for this whole file (introduced by defgroup at the beginning).

Current output of this file can be checked here: https://b-gent.github.io/PR2499/d8/d71/group__common__endian.html

@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc3 branch 4 times, most recently from 3faa077 to d0a5796 Oct 8, 2020
@b-gent
Copy link
Contributor Author

@b-gent b-gent commented Oct 15, 2020

In this PR, the following files are defined as high priority:

  • hashmap.h
  • fs.h
  • events.h
  • endian.h
  • debug.h
    Apart from debug.h , these have been reviewed by @sev- . I will clarify some of my doubts about debug directly.

The other files are lower priority and I am mostly just making typical edits in them. The only file where I am actually adding quite a few comments is ini-file.h. @sev- , please check those.

Editing doxygen comments in files:

- debug.h
- dialogs.h
- encoding.h
- endian.h
- error.h
- events.h
- fft.h
- file.h
- frac.h
- fs.h
- gui_options.h
- hash-ptr.h
- hashmap.h
- huffman.h
- ini-file.h

Plus some small changes in the config file.
@b-gent b-gent force-pushed the b-gent:improve_doxygen_doc3 branch from d0a5796 to 646f64f Oct 15, 2020
@deepcode-ci-bot
Copy link

@deepcode-ci-bot deepcode-ci-bot bot commented Oct 15, 2020

Congratulations 🎉. DeepCode analyzed your code in 2.923 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@b-gent b-gent changed the title [DNM] DOXYGEN: Doxygen improvements part 3 DOXYGEN: Doxygen improvements part 3 Oct 15, 2020
bool hasKey(const String &key, const String &section) const; /*!< Check whether the @p section has a @p key. */
bool getKey(const String &key, const String &section, String &value) const; /*!< Get the @p value of a @p key in a @p section. */
void setKey(const String &key, const String &section, const String &value); /*!< Assign a @p value to a @p key in a @p section. */
void removeKey(const String &key, const String &section); /*!< Remove a @p key from this @p section. */

This comment has been minimized.

@b-gent

b-gent Oct 16, 2020
Author Contributor

What does removeKey do?
a) Removes the key completely from the section/file.
b) Removes the current value from the key.

@criezy
criezy approved these changes Oct 29, 2020
Copy link
Member

@criezy criezy left a comment

I made some minor comments, but there is nothing crucial.

*
* The result has to be freed after use.
* The resulting string is ended by a character with value 0

This comment has been minimized.

@criezy

criezy Oct 29, 2020
Member

Is it a Github display artifact, or is there something wrong with the indentation on this line?

* The result has to be freed after use.
* The resulting string is ended by a character with value 0
* (C-like ending for 1 byte per character encodings, 2 zero bytes for UTF-16,
* 4 zero bytes for UTF-32). The result must be freed after usage.

This comment has been minimized.

@criezy

criezy Oct 29, 2020
Member

It might be worth mentioning that the result must be freed using free() and not delete[].

*
* The result has to be freed after use.
* This is a static version of the method above.
* The resulting string is ended by a character with value 0

This comment has been minimized.

@criezy

criezy Oct 29, 2020
Member

Same potential issue with the indentation as above.

* The string must be zero ended. Similar to strlen
* (could be used instead of strlen).
* (can be used instead of strlen).

This comment has been minimized.

@criezy

criezy Oct 29, 2020
Member

Maybe it should be clarified that this must be used instead of strlen for some encodings (such as UTF-16 and UTF-32) as strlen does not support multi-bytes encodings, with some exceptions (such as UTF-8).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.