RetroArch
RetroArch copied to clipboard
Add Doxygen-styled comments to parts of the libretro API
Description
This pull request will add Doxygen-styled comments to various parts of the libretro API to simplify their use. Even if HTML pages are not generated or hosted, Doxygen is common enough that IDEs will recognize it. Here's what it looks like in CLion:
The comments are written to adhere to SemBr for easier diffs and clearer sentence structure. Essentially, line breaks are treated like punctuation.
This PR will not:
- Violate C90. All additions come as
/** standard C90 comments */. - Change how types, functions, macros, etc. are declared.
- Generate documentation pages. This just adds comments.
- Document RetroArch internals. This is for the benefit of
libretro-common.
This PR, as of this writing, is only a sample. If @LibretroAdmin approves the general style, then I will proceed to document the rest of <libretro.h> (plus some other libretro-common APIs I've used) in the same format.
Reviewers
@LibretroAdmin @RobLoach
Status
The goal of this PR is to document ~all public-facing~ the parts of libretro-common that I've used. Here's where that stands:
- [ ] ~
array~ - [ ]
audio- [ ] ~
audio_mix.h~ - [ ] ~
audio_mixer.h~ - [ ]
audio_resampler.h - [x]
conversion- [x]
dual_mono.h - [x]
float_to_s16.h - [x]
s16_to_float.h
- [x]
- [ ] ~
dsp_filter.h~
- [ ] ~
- [ ]
boolean.h - [ ] ~
cdrom/cdrom.h~ - [x]
clamping.h - [ ]
compatNo need to write bespoke documentation for every single compatibility symbol, but libretro-specific limitations do need to be pointed out. It's enough to link to the documentation pages for whatever functions these reimplement.- [ ] ~
apple_compat.h~ - [x]
fnmatch.h - [ ] ~
fopen_utf8.h~ - [x]
getopt.h - [x]
ifaddrs.h - [x]
intrinsics.h - [ ] ~
msvc/stdint.h~ - [ ] ~
msvc.h~ - [x]
posix_string.h - [x]
strcasestr.h - [x]
strl.h - [ ] ~
zlib~
- [ ] ~
- [ ]
definesSame deal here, just link to the documentation pages that acutally describe these.- [ ] ~
cocoa_defines.h~ - [ ] ~
d3d_defines.h~ - [ ] ~
gx_defines.h~ - [ ] ~
ps3_defines.h~ - [ ] ~
ps4_defines.h~ - [ ] ~
psp_defines.h~
- [ ] ~
- [x]
dynamic/dylib.h - [ ]
encodings - [ ] ~
fastcpy.h~ - [x]
features/features_cpu.h - [ ]
file- [ ] ~
archive_file.h~ - [ ] ~
config_file.h~ - [ ] ~
config_file_userdata.h~ - [ ]
file_path.h - [ ] ~
nbio.h~
- [ ] ~
- [ ] ~
filters.h~ - [ ] ~
formats~- [ ] ~
cdfs.h~ - [ ] ~
image.h~ - [ ] ~
logiqx_dat.h~ - [ ] ~
m3u_file.h~ - [ ] ~
rbmp.h~ - [ ] ~
rjpeg.h~ - [ ] ~
rjson.h~ - [ ] ~
rjson_helpers.h~ - [ ] ~
rpng.h~ - [ ] ~
rtga.h~ - [ ] ~
rwav.h~ - [ ] ~
rxml.h~
- [ ] ~
- [ ]
gfx- [ ]
gl_capabilities.h - [ ] ~
math~- [ ] ~
matrix_3x3.h~ - [ ] ~
matrix_4x4.h~ - [ ] ~
vector_2.h~ - [ ] ~
vector_3.h~ - [ ] ~
vector_4.h~
- [ ] ~
- [ ]
scaler- [ ]
filter.h - [ ]
pixconv.h - [ ]
scaler.h - [ ]
scaler_int.h
- [ ]
- [ ] ~
video_frame.h~
- [ ]
- [ ]
glsym- [ ]
glsym.h - [ ]
glsym_es2.h - [ ]
glsym_es3.h - [ ]
glsym_gl.h - [ ]
rglgen.h - [ ]
rglgen_headers.h - [ ]
rglgen_private_headers.h - [ ] ~
switch~- [ ] ~
nx_gl.h~ - [ ] ~
nx_glsym.h~
- [ ] ~
- [ ]
- [ ] ~
libchdr~- [ ] ~
bitstream.h~ - [ ] ~
cdrom.h~ - [ ] ~
chd.h~ - [ ] ~
coretypes.h~ - [ ] ~
flac.h~ - [ ] ~
huffman.h~ - [ ] ~
libchdr_zlib.h~ - [ ] ~
lzma.h~ - [ ] ~
minmax.h~
- [ ] ~
- [ ] ~
libco.h~ - [ ]
libretro.hThis is the big one. Partly done, not complete. - [ ] ~
libretro_d3d.h~ - [ ] ~
libretro_dspfilter.h~ - [ ] ~
libretro_gskit_ps2.h~ - [ ]
libretro_vulkan.h - [ ]
lists- [ ] ~
dir_list.h~ - [ ] ~
file_list.h~ - [ ] ~
linked_list.h~ - [ ] ~
nested_list.h~ - [ ] ~
string_list.h~
- [ ] ~
- [ ] ~
lrc_hash.h~ - [ ] ~
math~- [ ] ~
complex.h~ - [ ] ~
float_minmax.h~ - [ ] ~
fxp.h~
- [ ] ~
- [ ] ~
media/media_detect_cd.h~ - [ ] ~
memalign.h~ - [ ] ~
memmap.h~ - [ ]
net- [ ] ~
net_compat.h~ - [ ]
net_http.h - [ ]
net_http_parse.h - [ ] ~
net_ifinfo.h~ - [ ] ~
net_socket.h~ - [ ] ~
net_socket_ssl.h~
- [ ] ~
- [ ] ~
playlists/label_sanitization.h~ - [ ]
queues- [x]
fifo_queue.h - [ ] ~
generic_queue.h~ - [ ] ~
message_queue.h~ - [x]
task_queue.h
- [x]
- [ ]
retro_assert.h - [x]
retro_common.h - [ ]
retro_common_api.h - [x]
retro_dirent.h - [x]
retro_endianness.h - [ ]
retro_environment.h - [x]
retro_inline.h - [ ] ~
retro_math.h~ - [x]
retro_miscellaneous.h - [x]
retro_timers.h - [ ]
rthreads- [x]
async_job.hThis has no definitions or users; I'm pretty sure this is obsolete functionality that was removed. - [x]
rthreads.h - [ ] ~
tpool.h~
- [x]
- [ ]
streams- [ ] ~
chd_stream.h~ - [x]
file_stream.h - [x]
file_stream_transforms.h - [ ] ~
interface_stream.h~ - [ ] ~
memory_stream.h~ - [x]
network_stream.h - [ ] ~
rzip_stream.h~ - [x]
stdin_stream.h - [ ] ~
trans_stream.h~
- [ ] ~
- [ ] ~
string/stdstring.h~ - [x]
time/rtime.h - [ ] ~
utils/md5.h~ - [ ]
vfs - [ ]
vulkan/vulkan_symbol_wrapper.h
I've been wanting doxygen in here FOREVER. Happy to help out on its transition, if the rest of the libretro team are :+1:
I've been wanting doxygen in here FOREVER. Happy to help out on its transition, if the rest of the libretro team are 👍
Great! How do you wanna divvy up the work? I'm happy to flesh out the docs for the environment calls plus any APIs that I've used so far. (That includes the VFS, task queue, and image scaler.) I'm also happy to proofread for spelling and clarity.
The APIs I've contributed already use Doxygen comments.
Oh, hey, it looks like there's a (default) Doxyfile in the repo already. I'll have to fine-tune it at some point.
I think my vision for this PR's roadmap could use a bit of clarification.
- I'd like to get doc comments with correct information written for all public-facing APIs defined in
libretro-common.- This step also includes refactoring "pseudo-Doxygen" to comments (like there was in
clamping.h, for instance) - For compatibility headers that fill in for missing C functions, not every symbol has to be documented. Only those that have libretro-specific limits like
getoptneed their own body copy. Otherwise, I think liberal use of@seetags (plus the occasional file-level doc comment) would suffice. For example, I wanna link to thismanpage forstrlcpysomewhere in<compat/strl.h>. Ditto for the OpenGL headers inglsym.
- This step also includes refactoring "pseudo-Doxygen" to comments (like there was in
- Once all the info is written and fact-checked, we can make a decision about how we actually expect to publish documentation, if at all. Remember, these doc comments will very likely be shown inline by IDEs; they will be useful even if we never run Doxygen.
- Once we make that decision, we can tweak the
Doxyfileand doc comment tags (groups, examples, etc.) to be in line with it. - After that happens, this PR will be good to merge in my mind. I don't want this to languish for too long, as there are other things I'd like to work on.
- Example: in the similar spirit of annotating declarations to make the APIs easier to use, I want to slap these annotations (or local equivalents) on various symbols. That'll be a later PR, though.
I've added a list of all the files that need to be documented. I would greatly appreciate some help checking these boxes off.
@RobLoach I had documentation I'd just written for RETRO_DEVICE that I hadn't committed yet. Maybe we should coordinate what exactly we're working on.
I have docs for network_stream.h, retro_endianness.h, retro_miscellaneous.h, file_path.h, file_stream.h, features_cpu.h, and strl.h in the oven. I will finish these files before I start any new ones. What about you?
Oops! Feel free to revert or force push your work 😉
Oops! Feel free to revert or force push your work 😉
It's fine, I can just merge them; we each covered some ground that the other didn't. That said, I still think we should coordinate which headers we're working on. I've given my near-term plans, what about yours?
I have just been touching up when using the API elsewhere. But could help with audio_mix stuff.
I can be sure to send PRs so that there aren't any conflict.
Sounds good. You could send them against either this branch on my fork, or just on RetroArch directly. I don't have a strong preference.
In the interest of getting this out the door, I'm going to reduce the scope of this PR to libretro.h and headers I've used.
That seems appropriate. We could take on the rest separately.
That seems appropriate. We could take on the rest separately.
Sounds good. At some point after this is merged, we could take a look at integrating the contents into the docs repo. I'm thinking that repo's workflow could do the following, in order:
- Clone the latest commit of RetroArch.
- Generate doc pages with MkDoxy or similar, consistent with the style of the rest of the doc pages.
I'm still working on this. I've been prioritizing melonDS DS, but I've been squeezing in some work here whenever I'm blocked on something. I've finished with most parts of libretro.h. Once I complete it (and one other header whose changes I haven't pushed yet), I'll go around requesting reviews.
In the interest of not letting this languish while I move on to something else, I've decided to call this branch done. Any gaps in it will be filled in future PRs. In the meantime, could I get some eyes on the contents?
@LibretroAdmin I believe this is good to go. Had another look through and there isn't any API or function definition that is changing, it's all documentation.
One exception; I do add a new enum here for RETRO_ENVIRONMENT_GET_AUDIO_VIDEO_ENABLE. However, the size and range of the enum's values are consistent with the hardcoded numbers in the original documentation, so I don't foresee any ABI problems.
@Jamiras can still review it.
Sounds good with me. The more eyes we have on this the better. When we decide to merge this forwards, it may be best to "Squash and merge" so that the git history doesn't get too crazy.