search

gridcf / gct-docs

Grid Community Toolkit documentation
http://gridcf.org/gct-docs/
1 stars 4 forks source link

Additonal corrections #3

Closed fscheiner closed 5 years ago

fscheiner commented 6 years ago

This includes additional corrections.

Please keep this PR open, because:

fscheiner commented 6 years ago

Version numbers

Mat has created {version} and {shortversion} attributes, i.e. vars that are expanded during the transformation process performed by AsciiDoctor from e.g. AsciiDoc to HTML.

I propose to change all occurrences of version numbers related to the GCT as a whole to these attributes respectively due to the following advantages:

Inter-document links

Inter-document links are currently mainly linking to the HTML version of an AsciiDoc file. As a result, they for example don't work "correctly" in the rendered AsciiDoc files on GitHub. AsciiDoctor allows something similar called "Inter-document Cross References" that will be adapted to the desired target format, i.e. link to HTML files in the HTML output only - IIUC.

But there are some specifics we need to anticipate: E.g. for fragments when relative paths are used for inter-doc. cross references (ICRs) - and I haven't yet found out how to use absolute paths for ICRs - the relative path to the referenced document needs to be chosen based on the location of the AsciiDoc file that includes the fragment and not based on the fragment's location. This makes a problem for fragments that are included in multiple documents which are itself located in different locations. Actually relative links in such fragments are a problem per se. So maybe a solution would be to not include fragments in multiple documents. I also don't see an advantage in this for "real" documentation, as we virtually replicate information on the website (same content at mutliple locations). Or can you think of a reason why this could be needed? It makes sense for example for the breadcrumb parts though.

What do you think?

msalle commented 6 years ago

Concerning the fragments / links, would it work to start the links with a / i.e. to make them absolute paths? See e.g. asciidoctor issue #844 And if so, can't we then restrict fragments to only use absolute links? Note/disclaimer: I'm absolutely not an expert in AsciiDoc(tor)

fscheiner commented 6 years ago

Me neither, I'm just testing various options. At least on GitHub the "start with a slash" method seems to work, see https://github.com/fscheiner/gct-docs/blob/c8fa6589dcaf3ddd3678f69d45806123dc087e34/myproxy/Cred_Mgmt_MyProxy_Usage_Statistics_Frag.adoc (here7). It didn't work for me locally, but I viewed the HTML files via a pseudo GVfs mount. And I don't know how it will look when rendered via GitHub pages. I also tried to use a variable, but GitHub doesn't really include fragment AsciiDoc files but just links them, so the value of the variable (EDIT: defined in https://raw.githubusercontent.com/fscheiner/gct-docs/c8fa6589dcaf3ddd3678f69d45806123dc087e34/myproxy/admin/index.adoc) is not known when the fragment is shown for itself.

fscheiner commented 6 years ago

Looks like the "start with a slash" method does not work when the HTML files are served from GitHub pages, see https://fscheiner.github.io/gct-docs/myproxy/admin/index.html#myproxy-usage (does serve the master branch which in my fork includes the test-xref modifications mentioned above). Most likely due to the anchor character preceding the path of the cross reference. :-/

msalle commented 6 years ago

Hmm, I think it interprets them as anchor names, not as paths. From asciidoc-reference-check (under supported-anchors-and-references) I would say you either need to add .adoc or perhaps use link instead of xref. Perhaps http://discuss.asciidoctor.org/Re-Link-versus-XRef-td327.html is also of any help?

fscheiner commented 6 years ago

I did some testing and had some further reading, thanks for the links. To me it now looks like the asciidoctor documentation I referenced earlier is not correct in regard to inter-document cross references, because all tests I did with both forms (<<[...]>> and xref:[...]) didn't behave as documented - or let's say not as expected by me. Just compare the raw file and the corresponding HTML output.

With the {outfilesuffix} variable as "suffix" to file names without file type suffix and a variable defined in the AsciiDoc file that includes a fragment pointing to the document root (as relative path, absolute paths don't seem to work), the link macro will work from fragments and allow to link to other documents and/or anchors in other documents for the HTML output served by GitHub pages with correct file type suffix. It doesn't work when viewing the AsciiDoc files itself on GitHub (see https://github.com/fscheiner/gct-docs/blob/master/myproxy/Cred_Mgmt_MyProxy_Usage_Statistics_Frag.adoc), as the variable pointing to the document root is not expanded there, as included fragments are not rendered as part of the files that includes them but just linked. So when viewing the fragment, used vars from files that include the fragment cannot be expanded. And we cannot define the document root var in the fragments, as they can be included by files in different locations.

So all in all no additional value to just directly link to the html files for the cases (1) viewing the AsciiDoc source on GitHub and (2) serving HTML from GitHub.

I haven't yet examined how this will behave for PDF output but will keep things as is for now for inter-document links until we find a better solution.

@matyasselmeci @ellert @msalle Any comments on the "Version numbers" part of https://github.com/gridcf/gct-docs/pull/3#issuecomment-417322340? I don't want to make these changes, commit them and only then get them rejected. :-)

matyasselmeci commented 6 years ago

Hi Frank, I support the version number changes except we need to make sure we don't change them in places like release notes and migration guides where the version number should be fixed.

fscheiner commented 6 years ago

[...] we need to make sure we don't change them in places like release notes and migration guides where the version number should be fixed.

Yeah, that's also my concern. I'll come up with a patch and promise to be careful :-D.

fscheiner commented 6 years ago

@matyasselmeci @msalle This PR is now complete with the changes related to the toolkit version number. HTML files are up to date with all prior changes. Please include it if you're satisfied with the changes.

To see how it could look in the future, please also check my mockup at https://fscheiner.github.io/gct-docs/. The mockup assumes version 7.0 is the latest available version of the GCT and is served from the master branch of my fork. This as you can only serve pages from (1) master, (2) gh-pages or (3) a /docs folder in the master branch and I didn't want to modify the gh-pages branch in my fork directly and consider the third option n/a. Please don't mind, the mockup is missing some last minutes changes I squashed into f36ec3e and 00585bd.

Interesting BTW that GitHub links those two commit hashes above in the context of gct-docs upstream, although they aren't yet included. ?-/

matyasselmeci commented 6 years ago

Could you update your mockup to add https://github.com/gridcf/gct-docs/pull/3/commits/00585bd9e96610628d1f17557c87c87a4e1937b4? It looks like the literal string {shortversion} is being put into the HTMLs instead of the actual version.

fscheiner commented 6 years ago

Oh, didn't notice this.

I just now checked /admin/install/appendix.html and to me it looks like AsciiDoctor cannot or does not expand variables, if a string with a variable is used in another variable. I.e. in the document mentioned above this comes from /admin/breadcrumb_frag.adoc:

ifdef::backend-html5[link:../../index.html[GCT] -> link:../index.html[Admin Manuals] -> {doctitle}]

...where {doctitle} is expanded to the title of the document that includes this fragment. But the title of /admin/install/appendix.adoc is defined as:

= GCT {shortversion} Installation Appendix =

and AsciiDoctor seems to use this unexpanded. To be sure I have to check other files which include breadcrumb fragments. That was highly unexpected.

UPDATE: Looking at a few other files in the mockup this doesn't seem to happen everywhere, so maybe I am wrong with my assumption above. I'll have to double-check things. Thanks for the pointer.

fscheiner commented 6 years ago

An update:

After double-checking things, my assumption from https://github.com/gridcf/gct-docs/pull/3#issuecomment-423628660 seems to be correct: Variables in document titles are not expanded when using the {doctitle} variable. Should we consider that a bug in AsciiDoctor?

The reason why it doesn't seem to happen everywhere in the mockup is that some documents use the version number verbatim, e.g. like in https://raw.githubusercontent.com/fscheiner/gct-docs/master/7.0/appendices/commands/index.adoc

Sadly this means, that we could not use variables with version numbers in document titles. Quickly scanning through https://github.com/gridcf/gct-docs/pull/3/commits/f36ec3e7c66313118609ebf78148443ba6b152a2, the majority of related changes unfortunately seems to happen in document titles. But using verbatim version numbers in the document titles and version number variables everywhere else seems to be inappropriate.

fscheiner commented 5 years ago

@matyasselmeci I dropped the version number changes for now to get the corrections into the documentation. HTML files were also updated to only include the corrections from 2018-08-30. So if there are no objections this PR can be merged in its current state.

I'll later come up with a PR for the GCT 6.2 documentation. There I'll follow the original plan to have:

  1. Versioned sub directories (like in https://github.com/fscheiner/gct-docs/tree/master)
  2. Versioned documentation - though done manually for now