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

Fixed the namespace and class icon issue and updated the Doxygen file. #750

Merged
merged 2 commits into from Dec 27, 2014

Conversation

Projects
None yet
6 participants
@eXpl0it3r
Member

eXpl0it3r commented Dec 12, 2014

While generating the 2.2 documentation for the website, I noticed that the Doxygen and the CSS files were outdated, thus I've made some adjustments.

While doing so I've now also link the JavaScript files in the header, thus the JavaScript powered bits in the documentation now work.

Additionally I changed the file ending to .html instead of .htm and made everything use UTF-8.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 16, 2014

Member

Feel free to take a look, test and/or comment. 😉

Member

eXpl0it3r commented Dec 16, 2014

Feel free to take a look, test and/or comment. 😉

Show outdated Hide outdated doc/doxygen.css
@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 16, 2014

Member

The diff for doc/doxyfile.in is not shown online (too big). What's the changes? Line endings mostly?

Member

mantognini commented Dec 16, 2014

The diff for doc/doxyfile.in is not shown online (too big). What's the changes? Line endings mostly?

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 16, 2014

Member

Most of the changes are related to the comments. I took the default doxygen file (provided by the doxygen wizard) and applied the old changes, as well as removed a few deprecated/obsolete options. Thus eliminating some of the errors you'd get when generating the docs.

The main change is however, that the namespace and class icons aren't images anymore, thus they needed some additional CSS styling. Plus the JavaScript files were never linked in the HTML, thus the few JavaScript features (tool tip, level collapsing, etc.) should now work.

Member

eXpl0it3r commented Dec 16, 2014

Most of the changes are related to the comments. I took the default doxygen file (provided by the doxygen wizard) and applied the old changes, as well as removed a few deprecated/obsolete options. Thus eliminating some of the errors you'd get when generating the docs.

The main change is however, that the namespace and class icons aren't images anymore, thus they needed some additional CSS styling. Plus the JavaScript files were never linked in the HTML, thus the few JavaScript features (tool tip, level collapsing, etc.) should now work.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 16, 2014

Member

Alright. I can't test right now but if the doc for 2.2 on the test website was produced by this, I see only one minor issue: on the file list page, the files are not viewable (no link) but we can access them from a class dedicated page.

Member

mantognini commented Dec 16, 2014

Alright. I can't test right now but if the doc for 2.2 on the test website was produced by this, I see only one minor issue: on the file list page, the files are not viewable (no link) but we can access them from a class dedicated page.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 16, 2014

Member

You're right, the file/doc icon is missing as well. I'll investigate it, nice catch!

Member

eXpl0it3r commented Dec 16, 2014

You're right, the file/doc icon is missing as well. I'll investigate it, nice catch!

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 16, 2014

Member

It wasn't an issue with the CSS file, but rather the website. Path images in CSS are relative to the style sheet and not the HTML file it gets included into. It has been fixed on the test site.

Member

eXpl0it3r commented Dec 16, 2014

It wasn't an issue with the CSS file, but rather the website. Path images in CSS are relative to the style sheet and not the HTML file it gets included into. It has been fixed on the test site.

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Dec 18, 2014

Member

Looks good 👍 🐈

Member

TankOs commented Dec 18, 2014

Looks good 👍 🐈

@eXpl0it3r eXpl0it3r added this to the 2.3 milestone Dec 20, 2014

@eXpl0it3r eXpl0it3r self-assigned this Dec 20, 2014

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 20, 2014

Member

This PR has been added to my merge list, meaning it will be merged very soon, unless someone raises any concerns.

Member

eXpl0it3r commented Dec 20, 2014

This PR has been added to my merge list, meaning it will be merged very soon, unless someone raises any concerns.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 21, 2014

Member

Now that I see the diff on my computer I have a few comments. Sorry I couldn't do it earlier. First, a very minor thing: there are trailing spaces. Beside that:

  • PROJECT_LOGO and PROJECT_BRIEF: since those are new, why not take advantage of them (and not leave them blank)?
  • MARKDOWN_SUPPORT: I haven't checked but maybe it's better to set it to NO since we were not relying on it before and therefore it might break stuff.
  • What about using EXAMPLE_PATH?
  • DOCSET_FEEDNAME, DOCSET_BUNDLE_ID, DOCSET_PUBLISHER_ID and DOCSET_PUBLISHER_NAME could be updated as well. (I need to update them anyway for the Dash docset so I'll provide here some values later.)
  • What about enabling SEARCHENGINE? It can be quite useful. (I'm talking about the JS version, not the server based one.) Or do we rely only on GENERATE_HTMLHELP?
Member

mantognini commented Dec 21, 2014

Now that I see the diff on my computer I have a few comments. Sorry I couldn't do it earlier. First, a very minor thing: there are trailing spaces. Beside that:

  • PROJECT_LOGO and PROJECT_BRIEF: since those are new, why not take advantage of them (and not leave them blank)?
  • MARKDOWN_SUPPORT: I haven't checked but maybe it's better to set it to NO since we were not relying on it before and therefore it might break stuff.
  • What about using EXAMPLE_PATH?
  • DOCSET_FEEDNAME, DOCSET_BUNDLE_ID, DOCSET_PUBLISHER_ID and DOCSET_PUBLISHER_NAME could be updated as well. (I need to update them anyway for the Dash docset so I'll provide here some values later.)
  • What about enabling SEARCHENGINE? It can be quite useful. (I'm talking about the JS version, not the server based one.) Or do we rely only on GENERATE_HTMLHELP?
@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 21, 2014

Member

Also I get some warnings about CLANG_ASSISTED_PARSING and CLANG_OPTIONS: doxygen suggests to remove them from the doxyfile since this option was not enabled at compile time.

Member

mantognini commented Dec 21, 2014

Also I get some warnings about CLANG_ASSISTED_PARSING and CLANG_OPTIONS: doxygen suggests to remove them from the doxyfile since this option was not enabled at compile time.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 22, 2014

Member

PROJECT_LOGO

With the PROJECT_LOGO tag one can specify an logo or icon that is included in
the documentation. The maximum height of the logo should not exceed 55 pixels
and the maximum width should not exceed 200 pixels. Doxygen will copy the logo
to the output directory.

We could think about that, but for this branch I don't want to include these changes.

PROJECT_BRIEF

Using the PROJECT_BRIEF tag one can provide an optional one line description
for a project that appears at the top of each page and should give viewer a
quick idea about the purpose of the project. Keep the description short.

I don't think we want that on every page.

MARKDOWN_SUPPORT

EXAMPLE_PATH

The EXAMPLE_PATH tag can be used to specify one or more files or directories
that contain example code fragments that are included (see the \include
command).

As far as I know we're not using the \include command for the examples shown in the documentation, thus such a path doesn't even exist.

DOCSET_*

Can you provide the values?

SEARCHENGINE

No idea, it's been disabled before and by default GENERATE_HTMLHELP is activated.

CLANG_ASSISTED_PARSING and CLANG_OPTIONS

Since CLANG_ASSISTED_PARSING is set to NO by default, this shouldn't really matter. Can you remove it and see if it still generates a warning? If not we could remove it.

Member

eXpl0it3r commented Dec 22, 2014

PROJECT_LOGO

With the PROJECT_LOGO tag one can specify an logo or icon that is included in
the documentation. The maximum height of the logo should not exceed 55 pixels
and the maximum width should not exceed 200 pixels. Doxygen will copy the logo
to the output directory.

We could think about that, but for this branch I don't want to include these changes.

PROJECT_BRIEF

Using the PROJECT_BRIEF tag one can provide an optional one line description
for a project that appears at the top of each page and should give viewer a
quick idea about the purpose of the project. Keep the description short.

I don't think we want that on every page.

MARKDOWN_SUPPORT

EXAMPLE_PATH

The EXAMPLE_PATH tag can be used to specify one or more files or directories
that contain example code fragments that are included (see the \include
command).

As far as I know we're not using the \include command for the examples shown in the documentation, thus such a path doesn't even exist.

DOCSET_*

Can you provide the values?

SEARCHENGINE

No idea, it's been disabled before and by default GENERATE_HTMLHELP is activated.

CLANG_ASSISTED_PARSING and CLANG_OPTIONS

Since CLANG_ASSISTED_PARSING is set to NO by default, this shouldn't really matter. Can you remove it and see if it still generates a warning? If not we could remove it.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 22, 2014

Member

PROJECT_LOGO

Alright

PROJECT_BRIEF

I didn't realise it was for all pages... so, yes, we don't want that. :-)

EXAMPLE_PATH

I was rushing a bit too fast by fear you'd merge it while I was reviewing it. It seems I've oversighted this too.. sorry.

DOCSET_*

I'll use those values for the docset:

DOCSET_FEEDNAME        = "SFML Documentation"
DOCSET_BUNDLE_ID       = org.sfml-dev.SFML
DOCSET_PUBLISHER_ID    = org.sfml-dev.SFML
DOCSET_PUBLISHER_NAME  = SFML

SEARCHENGINE

GENERATE_HTMLHELP is only available on Windows. But I tested SEARCHENGINE = YES and it's a mess: the non-server based search doesn't work and messes up with the css.. so let's forget it.

CLANG_ASSISTED_PARSING and CLANG_OPTIONS

I can confirm that removing those entries also removes the warning.

Member

mantognini commented Dec 22, 2014

PROJECT_LOGO

Alright

PROJECT_BRIEF

I didn't realise it was for all pages... so, yes, we don't want that. :-)

EXAMPLE_PATH

I was rushing a bit too fast by fear you'd merge it while I was reviewing it. It seems I've oversighted this too.. sorry.

DOCSET_*

I'll use those values for the docset:

DOCSET_FEEDNAME        = "SFML Documentation"
DOCSET_BUNDLE_ID       = org.sfml-dev.SFML
DOCSET_PUBLISHER_ID    = org.sfml-dev.SFML
DOCSET_PUBLISHER_NAME  = SFML

SEARCHENGINE

GENERATE_HTMLHELP is only available on Windows. But I tested SEARCHENGINE = YES and it's a mess: the non-server based search doesn't work and messes up with the css.. so let's forget it.

CLANG_ASSISTED_PARSING and CLANG_OPTIONS

I can confirm that removing those entries also removes the warning.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 22, 2014

Member

I just realised that the version of SFML is not present (or well hidden!) in the doc. On sfml-dev.org it's alright since it's embeded in the website with a nice title. But not on the offline version.

Something like this patch could fix it. Basically, header.html is renamed into header.html.in, the CMakeLists.txt is updated to configure the previous file and doyxfile is also updated to take it into account.

Member

mantognini commented Dec 22, 2014

I just realised that the version of SFML is not present (or well hidden!) in the doc. On sfml-dev.org it's alright since it's embeded in the website with a nice title. But not on the offline version.

Something like this patch could fix it. Basically, header.html is renamed into header.html.in, the CMakeLists.txt is updated to configure the previous file and doyxfile is also updated to take it into account.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 22, 2014

Member

I like the idea of including the version. The question I've to ask myself then is, what is this "intermediate" version then? I mean if we add new features and bugfixes and someone builds SFML's documentation, it'd say SFML 2.2, even though it's SFML 2.3-dev or something.

Member

eXpl0it3r commented Dec 22, 2014

I like the idea of including the version. The question I've to ask myself then is, what is this "intermediate" version then? I mean if we add new features and bugfixes and someone builds SFML's documentation, it'd say SFML 2.2, even though it's SFML 2.3-dev or something.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Dec 23, 2014

Member

The question I've to ask myself then is, what is this "intermediate" version then?

The same question applies to the version number in Config.hpp, so I'd say just do the same (update the number only for the official release).

Member

LaurentGomila commented Dec 23, 2014

The question I've to ask myself then is, what is this "intermediate" version then?

The same question applies to the version number in Config.hpp, so I'd say just do the same (update the number only for the official release).

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 23, 2014

Member

I'd say we keep it like Laurent said for 2.x. But for 3.x it would make sense to add -dev (or even the date of the doc build) because the dev process will last for quite a while so we can avoid any confusion, especially since the API will often change. For 2.x, it's not worth it since it's pretty stable.

Member

mantognini commented Dec 23, 2014

I'd say we keep it like Laurent said for 2.x. But for 3.x it would make sense to add -dev (or even the date of the doc build) because the dev process will last for quite a while so we can avoid any confusion, especially since the API will often change. For 2.x, it's not worth it since it's pretty stable.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 24, 2014

Member

@mantognini About your patch: Do we really need to rename the header HTML file? Are there any advantages?
Since the *.in file is used for Doxygen settings, it might be confusing if the HTML file used *.in as well.
Otherwise, feel free to push your ideas directly to this branch.

Member

eXpl0it3r commented Dec 24, 2014

@mantognini About your patch: Do we really need to rename the header HTML file? Are there any advantages?
Since the *.in file is used for Doxygen settings, it might be confusing if the HTML file used *.in as well.
Otherwise, feel free to push your ideas directly to this branch.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 24, 2014

Member

It's solely for consistency with other configured file by cmake. (doxygen don't use *.in file if I'm not mistaken.)

I don't have access to my computer for a few days but I will push it then.

Member

mantognini commented Dec 24, 2014

It's solely for consistency with other configured file by cmake. (doxygen don't use *.in file if I'm not mistaken.)

I don't have access to my computer for a few days but I will push it then.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Dec 26, 2014

Member

Done. ;-)

Member

mantognini commented Dec 26, 2014

Done. ;-)

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Dec 26, 2014

Member

Looks good to me! 👍

SFML 2.2.0

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Dec 26, 2014

Looks good to me! 👍

SFML 2.2.0

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Dec 27, 2014

Member

👍

Member

binary1248 commented Dec 27, 2014

👍

@eXpl0it3r eXpl0it3r merged commit b0d6c2b into master Dec 27, 2014

@eXpl0it3r eXpl0it3r deleted the bugfix/doxygen branch Dec 27, 2014

@eXpl0it3r eXpl0it3r referenced this pull request Jan 6, 2015

Closed

Update documentation #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment