Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#2864 closed enhancement (fixed)

Add link to source code in the documentation pages.

Reported by: pmav99 Owned by: lucadelu
Priority: normal Milestone: 7.2.0
Component: Docs Version: svn-releasebranch72
Keywords: open science, access Cc: grass-dev@…
CPU: Unspecified Platform: Unspecified

Description

I don't know how easy it is to implement this, but I think that it would be really helpful if the online documentation had links to the source code.

E.g. To add a link in this page to this one.

Change History (22)

comment:1 Changed 4 years ago by wenzeslaus

Keywords: open science access added
Milestone: 7.0.37.0.4
Version: unspecifiedsvn-releasebranch70

Makes sense to me. For example Whitebox has a View Code button in the the GUI:

https://whiteboxgeospatial.files.wordpress.com/2014/05/whiteboxclumpdialog.png

The documentations seems a better place but I can see a reason why they decided for GUI (similarly to Modify Help Entry button).

For start you can try to add it to the footer of the manual page. The tools/mkhtml.py is where it is implemented.

comment:2 in reply to:  1 ; Changed 4 years ago by lucadelu

Replying to wenzeslaus:

The documentations seems a better place but I can see a reason why they decided for GUI (similarly to Modify Help Entry button).

For start you can try to add it to the footer of the manual page. The tools/mkhtml.py is where it is implemented.

Implemented in trunk r67561. Let me know if you like it and if I can backport it.

comment:3 in reply to:  2 Changed 4 years ago by martinl

Replying to lucadelu:

Implemented in trunk r67561. Let me know if you like it and if I can backport it.

Let's wait for 7.0.4

comment:4 in reply to:  2 ; Changed 4 years ago by martinl

Replying to lucadelu:

Implemented in trunk r67561. Let me know if you like it and if I can backport it.

I would suggest to move 'Source code' item at the end of the line (after 'Full index').

Moreover this line contains index links (so something generic which doesn't depend on the given module). From this POV 'source code' (linked to the module) is strange creature here. On the other hand I don't have better idea where to put it.

BTW, the commit contains several non-necessary changes (line indentation), we should try to avoid them.

comment:5 in reply to:  4 Changed 4 years ago by martinl

Replying to martinl:

Moreover this line contains index links (so something generic which doesn't depend on the given module). From this POV 'source code' (linked to the module) is strange creature here. On the other hand I don't have better idea where to put it.

or something like (two lines)

 Source code
 Main index | Raster index | Topics index | Keywords index | Graphical index | Full index 

?

Last edited 4 years ago by martinl (previous) (diff)

comment:6 in reply to:  4 ; Changed 4 years ago by lucadelu

Replying to martinl:

Replying to lucadelu:

Implemented in trunk r67561. Let me know if you like it and if I can backport it.

I would suggest to move 'Source code' item at the end of the line (after 'Full index').

Moreover this line contains index links (so something generic which doesn't depend on the given module). From this POV 'source code' (linked to the module) is strange creature here. On the other hand I don't have better idea where to put it.

I also don't like the place, I would like to see it in the SEE ALSO paragraph or something similar but it is to much work. Maybe the two lines is better

BTW, the commit contains several non-necessary changes (line indentation), we should try to avoid them.

yes we should avoid to leave empty lines with spaces and start to follow PEPs ;-) Should I first commit only the cleanup and after also the changes in the code?

My editor, correctly, is removing spaces in blank lines automatically.

comment:7 in reply to:  6 Changed 4 years ago by martinl

Replying to lucadelu:

yes we should avoid to leave empty lines with spaces and start to follow PEPs ;-) Should I first commit only the cleanup and after also the changes in the code?

but that it should be a separate commit...

comment:8 Changed 4 years ago by pmav99

Thank you all, As far as I am concerned, the two lines seems cleaner to me. Now with regard to the link itself.

  1. I am not sure if hard-coding the link to trunk is the best option. Since the documentation of multiple GRASS versions are online (i.e. 6.4, 7.0, 7.1) I think that the link should point to the respective branch.
  2. Since I guess that many users like myself don't have any particular knowledge of the GRASS code structure, in order to make this even more useful I think that the links themselves should directly point to the directory that contains the command in question.

I think that these two goals are not too hard to be achieved. E.g. assuming that we know on which branch we are something like this would give as a dictionary mapping each command to the correct link (you need to execute it from the repository's root directory.

import os, fnmatch, urlparse

grass_branch = 70
grass_base_dir = "."
branch_source_url = {
    64: "https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_6_4/",
    70: "https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/",
    71: "https://trac.osgeo.org/grass/browser/grass/trunk/",
}
base_url = branch_source_url[grass_branch]

grass_command_paths = {}
grass_command_pattern = r'[a-zA-Z0-9]*.[a-zA-Z0-9]*'
for root, dirs, files in os.walk(grass_base_dir):
    for name in dirs:
        if fnmatch.fnmatch(name, grass_command_pattern):
            grass_command_paths[name] = urlparse.urljoin(base_url, os.path.join(root, name))

from pprint import pprint
pprint(grass_command_paths)

Expected output:

{'English.lproj': 'https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/macosx/app/English.lproj',
 'MainMenu.nib': 'https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/macosx/app/English.lproj/MainMenu.nib',
 'd.barscale': 'https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/display/d.barscale',
 'd.colorlist': 'https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/display/d.colorlist',
 'd.colortable': 'https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/display/d.colortable',
 'd.correlate': 'https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_0/scripts/d.correlate',
 # ...
}

Mind you that there are a few false positives, but they should not be a problem since pgm in mkhtml.py will not match them. Of course, making the regex stricter is always possible.

comment:9 in reply to:  8 Changed 4 years ago by lucadelu

Replying to pmav99:

Thank you all, As far as I am concerned, the two lines seems cleaner to me. Now with regard to the link itself.

  1. I am not sure if hard-coding the link to trunk is the best option. Since the documentation of multiple GRASS versions are online (i.e. 6.4, 7.0, 7.1) I think that the link should point to the respective branch.

It is hard-coded only because the idea is to have a url for each version. there is a variable and it is changed only once. Using the dictionary mapping it is more or less the same, when a new version will come it should be added to the dictionary. With only a variable there is less code and it should be a little bit faster.

  1. Since I guess that many users like myself don't have any particular knowledge of the GRASS code structure, in order to make this even more useful I think that the links themselves should directly point to the directory that contains the command in question.

If I understand correctly it is already like that

comment:10 Changed 4 years ago by pmav99

If I understand correctly it is already like that.

I am afraid that it does not work for the scripts commands.

comment:11 in reply to:  10 ; Changed 4 years ago by wenzeslaus

Replying to pmav99:

If I understand correctly it is already like that.

I am afraid that it does not work for the scripts commands.

scripts dir is one exception. Another are C modules living in the same directory because they share a lot of code. Now I can think of r3mapcalc and r3.colors. Addons are of course yet another challenge.

We should definitively wait with backport before all these issues are resolved. I almost changed the milestone to 7.1 instead of 7.0.4.

Last edited 4 years ago by wenzeslaus (previous) (diff)

comment:12 in reply to:  11 Changed 4 years ago by lucadelu

Replying to wenzeslaus:

Replying to pmav99:

If I understand correctly it is already like that.

I am afraid that it does not work for the scripts commands.

scripts dir is one exception. Another are C modules living in the same directory because they share a lot of code. Now I can think of r3mapcalc and r3.colors. Addons are of course yet another challenge.

scripts should be solved in r67565 and r67566.

I was already thinking to Addons, I'll investigate next days.

Do we have a list of modules like r3.mapcalc and r3.colors?

We should definitively wait with backport before all these issues are resolved. I almost changed the milestone to 7.1 instead of 7.0.4.

I don't know probably it is not so much work, I should be able to fix it before 7.0.4.

comment:13 Changed 4 years ago by pmav99

I am afraid that this still does not work with scripts like those in raster/r.li/r.li.* and imagery/i.ortho.photo/i.ortho.*. Anyway, instead of adding special cases all over the place, wouldn't a single function be a cleaner solution?

This should also work with addons (at least I don't see why it wouldn't). Efficiency is not really relevant in documentation generation code and even if it is it should be addressed as a separate issue (get it working first, fast later).

import os
import urlparse


def get_source_code_url(pgm, base_url, root_path):
    """
    Return a url to the source code of `pgm`.

    If the link cannot be constructed, return the url to the repo's root directory.

    Parameters
    ----------
    pgm: str
        The name of the GRASS command.
    base_url: str
        The base url of the online source code browser.
    root_path: str
        The relative path to repository's root directory.

    """
    url = base_url
    for root, dirs, files in os.walk(root_path):
        for name in dirs:
            if name == pgm:
                pgm_path = os.path.join(root, name)
                url = urlparse.urljoin(base_url, pgm_path)
                return url
    return url

comment:14 in reply to:  13 ; Changed 4 years ago by lucadelu

Replying to pmav99:

I am afraid that this still does not work with scripts like those in raster/r.li/r.li.* and imagery/i.ortho.photo/i.ortho.*. Anyway, instead of adding special cases all over the place, wouldn't a single function be a cleaner solution?

Should be done with r67578, I didn't use your function and I did something really simple and it should work for every module.

For addons how it work the compilation of documentation?

comment:15 in reply to:  4 ; Changed 4 years ago by neteler

Replying to martinl:

I would suggest to move 'Source code' item at the end of the line (after 'Full index').

...

On the other hand I don't have better idea where to put it.

Suggestion: For now we put it *above* the footer line. It could even have a <h2>SOURCE CODE</h2> entry there. It is then still below the "Last changed" line but at least associated to the respective module text.

comment:16 in reply to:  14 Changed 4 years ago by pmav99

Replying to lucadelu:

I didn't use your function

No problem, If I were you I would still put those lines in a function though. No reason not to follow best practises. Anyway, thank you for your efforts.

comment:17 Changed 3 years ago by martinl

Milestone: 7.0.47.0.5

comment:18 in reply to:  15 Changed 3 years ago by neteler

Replying to neteler:

Suggestion: For now we put it *above* the footer line.

... I still think that this would be definitely better.

Or: It could even have an own <h2>SOURCE CODE</h2> entry at page bottom below "AUTHOR(S)". Then it would be still below the "Last changed" line but at least visually associated to the respective module text and show up in the TOC.

comment:19 Changed 3 years ago by martinl

Milestone: 7.0.57.2.0

comment:20 Changed 3 years ago by neteler

Cc: grass-dev@… added
Owner: changed from grass-dev@… to lucadelu
Version: svn-releasebranch70svn-releasebranch72

comment:21 Changed 3 years ago by lucadelu

Resolution: fixed
Status: newclosed

In 69290:

documentation: moved source code link and add revision log link, backport r69283 r69289, fixed #2864

comment:22 Changed 3 years ago by neteler

In 70192:

documentation: add link to source code (see #2864) (backport from relbranch72 in order to get the link also into addon manual pages: r69290 + r67566)

Note: See TracTickets for help on using tickets.