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

Split up macros on PODViewer index page. #2615

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

somiaj
Copy link
Contributor

@somiaj somiaj commented Nov 6, 2024

This splits up the macros on the PODViwer index page to be grouped by their location in the pg/macros directory. In addition macros are listed before the libraries. The two macros not in a subdirectory, PG.pl and PGcourse.pl, are grouped with the pod in pg/doc. This makes finding the POD for a specific macro easier.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two issues with this pull request.

The first is that this changes how the POD is organized inconsistent with how the pod generated by the bin/dev_scripts/generate-ww-pg-pod.pl script is organized (that is the pod displayed at https://webwork.maa.org/pod/).

The second is that the reorganization of the files into sub directories was not intended to be a visible categorization of macros. At least not initially. It was done more for development purposes to keep the pg/macros directory from being a completely unorganized mess with one long list of files. The organization that was done was not the most intuitive in terms of what macros are actually used for. For example, all macros that started with "parser" were just put in the "parsers" subdirectory, and all macros that started with "context" into the "contexts" subdirectory. Sorting purely by what the macro started with is not the best way to do that. The intent was to sort things better eventually. For example. the parserGraphTool.pl macro was later moved to the "graph" subdirectory. I am not sure that the organization is ready to be publicized in this way.

So this needs discussion first.

@somiaj
Copy link
Contributor Author

somiaj commented Nov 11, 2024

I think we are already publicizing the macro organization this way. The macros are currently sorted by what directory they are in, further that directory is printed in the link name. So knowing what directory a macro is in helps one find it in the long list of macros on the POD index (this mostly just splits that list apart). Though I do welcome more discussion on this to have a way that users can easily find the POD of a macro they are using or find a new macro they may want to use.

@drgrice1
Copy link
Member

We are currently listing them with their directory for ease of location. The sorting will then list those in the same directory together. That is not exactly the same thing as a categorization. As I said, this needs discussion. In any case, my first point is more pressing. The dev script will need to be updated to match. That is if we decide we want this organization.

@somiaj
Copy link
Contributor Author

somiaj commented Nov 22, 2024

Updated dev_scripts/generate-ww-pg-pod.pl to match splitting up the macros link.

This splits up the macros on the PODViwer index page to be
grouped by their location in the `pg/macros` directory. In addition
macros are listed before the libraries. The two macros not in a
subdirectory, `PG.pl` and `PGcourse.pl`, are grouped with the pod
in `pg/doc`. This makes finding the POD for a specific macro easier.

Also update `generate-ww-pg-pd.pl` to match the changes for the
POD included on the webwork wiki.
@drgrice1
Copy link
Member

drgrice1 commented Nov 24, 2024

I don't really like that the macro subdirectories are hard coded into this (in both templates/ContentGenerator/PODViewer.html and the generate-ww-pod.pl script). That is a maintenance hassle. Every time a directory is added, removed, or renamed, then this code has to be updated.

@somiaj
Copy link
Contributor Author

somiaj commented Nov 24, 2024

I realized that, but also felt that alphabetical order wasn't the best way to order the macros by, so I went with this so the order can be better controlled. So to me the question is the hassle worth having a different ordering than alphabetical.

@drgrice1
Copy link
Member

Actually, I think it would be better to go with alphabetical anyway. I didn't realize you were ordering them in a particular way. Why have you chosen that order anyway? The only subdirectory that would be nice to move down is the "deprecated" directory. There is no real reason to order the rest.

@somiaj
Copy link
Contributor Author

somiaj commented Nov 24, 2024

Okay, updated to not have a pre defined list for the order and macros included. There is still a hash of macro names that will include the translated and/or custom name, so "User Interface" is shown instead of "ui". Any new or renamed directory not included in that hash will still be included, just won't have a custom name attached to it.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better now. At least all directories will be represented. I am still not sure of the benefits of providing alternate labels for the directories and even translating them. That tacitly assumes the directory categorization has more meaning than the haphazard sorting of macros that it mostly is at this point.

bin/dev_scripts/PODtoHTML.pm Outdated Show resolved Hide resolved
bin/dev_scripts/PODtoHTML.pm Outdated Show resolved Hide resolved
To keep from having to update the list each time a macro directory
is added/removed/changed, just sort the macros by their directory
alphabetically. There is still a hash of macro names to allow both
translated or custom names instead of using the directory as its
name, but any directory that isn't in the list will also be included
without updating the list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants