Separate Utils.pm into smaller Utils/*.pm modules.

Set handling methods are now in `WeBWorK::Utils::Sets`. This module exports the methods:

format_set_name_internal
format_set_name_display
grade_set
grade_gateway
grade_all_sets
is_restricted
get_test_problem_position
list_set_versions

Note that list_set_version was the only method in WeBWorK::Utils::Grades which is now deleted.

JITAR problem methods are in `WeBWorK::Utils::JITAR`. This module exports the methods:

seq_to_jitar_id
jitar_id_to_seq
is_jitar_problem_hidden
is_jitar_problem_closed
jitar_problem_finished
jitar_problem_adjusted_status
prob_id_sort

File system interaction methods are in `WeBWorK::Utils::Files`. This module exports the methods:

surePathToFile
readFile
listFilesRecursive
path_is_subdir

Note that the methods createDirectory, makeTempDirectory, removeTempDirectory that were in WeBWorK::Utils have been deleted, since they are not used. The readFile method has been updated to use slurp from Mojo::File. The surePathToFile method now uses make_path from Mojo::File. The Utils readDirectory usage has been replaced with Mojo::File::list usage, and the readDirectory method removed. The read_dir and getCSVList methods of WeBWorK::Utils::Instructor which used readDirectory have also been removed, and Mojo::File::list used directly where those methods were used previously.

A bug in the listFilesRecursive method was fixed that prevented it from recursing into symbolic links to directories. Furthermore, the method was completely rewritten to use File::Find which should be faster than the recursive method, and is certainly cleaner codewise.

The templates/ContentGenerator/Instructor/FileManager/delete.html.ep file has been switched from using the the readDirectory and listFilesRecursive methods to using the list and list_tree methods of Mojo::File. Now that listFilesRecursive correctly recurses into symbolic links, this method reports the incorrect number of files being deleted in directories containing symbolic links. The actual deletion does not follow symbolic links, so the files inside symbolic links should not be counted. The list_tree method does not follow symbolic links.

With the above change, the only place that listFilesRecursive is actually used is in templates/ContentGenerator/Instructor/ProblemSetDetail.pm to list set and hardcopy headers. Since listFilesRecursive is fixed, you will now see set headers that are inside symbolic links in a courses templates directory. Note that this does NOT include set headers that are in any library listed in $courseFiles{problibs} (Library, Contrib, and capaLibrary by default) since those directories are pruned. That is true regardless of if those are directories or symbolic links.

The get_library_sets and get_library_pgs methods of WeBWorK::ContentGenerator::Instructor::SetMaker have been renamed to get_course_pg_dirs and get_pg_files_in_dir to more accurately reflect what they really do. They have also been heavily rewritten to effectively utilize the Mojo::File::list method and the features of Mojo::File and Mojo::Collection. Note that issues were seen with those methods that were fixed. Namely there were cases where pg files in a courses templates directory could not be accessed from the library browser as they should be.

Rather than updating several of the unused methods in WebworkWebservice::LibraryActions, those methods have been removed. The methods that remain are the only ones used by webwork2.

Logging methods are provided by `WeBWorK::Utils::Logs`. This module exports the methods:

writeCourseLog
writeLog
writeTimingLogEntry

Note that there is no longer a method writeCourseLogGivenTime. Instead the writeCourseLog method accepts an optional $time argument which defaults to the current time. To be able to do that the accepted $message argument can not be a slurpy parameter. That was never actually used anyway though (and would not have worked well if it was).

Date/Time handling methods are provided by `WeBWorK::Utils::DateTime`. This module exports the methods:

before
between
after
formatDateTime
getDefaultSetDueDate
parseDateTime
timeToSec

Other related things

The constituency_hash, dequote, fisher_yates_shuffle, list2hash, pretty_print_rh, and ref2string methods in WeBWorK::Utils were deleted since they are not used.

The rest of the WeBWorK::Utils module is cleaned up. Signatures are now used in that file, as well as the new files.

POD has been added for all methods in these files.

Also there is some inclusion clean up.

Also add follow_skip => 2 to the File::Find find call in the WeBWorK::Utils::Instructor getDefList method. If two symbolic links point to the same location, then find will die when it is about to process a file pointed to by the second link because it has already processed that file. It shouldn't die. Setting follow_skip => 2 makes find ignore the duplicate. This was discovered while fixing the listFilesRecursive method.

The bin/crypt_passwords_in_classlist.pl script was rewritten to be like the others. That is it uses methods from the webwork2 perl lib but still can be executed from anywhere. This means the cryptPassword method in WeBWorK::Utils is only in one place, and does not need to be kept in sync with a copy of it in this script.

I found that the file listings with the "Course Files" button in the library browser was not quite correct yet. Those issues are now fixed.

I have a question though.

The way that it was working is that if a directory had the name dir_name, that directory contained a single pg file named dir_name.pg and there were other files in that directory (not counting .bak, .tmp, files ending in ~, files ending in Header.pg, or files ending in -text.pg), then the directory would not be listed, and the pg file in that directory would be displayed when the parent directory is viewed. That behavior is preserved with this pull request.

However, previously if a directory named dir_name had a single pg file named dir_name.pg and no other files, then the directory would still not be listed, and the file also would not be displayed when the parent directory is viewed. That is changed as I mentioned, and now the directory is listed and the file can be viewed by viewing its directory.

My question is, should this stop checking for other files and always "combine up" when there is a single pg file in a directory by the same name (without .pg)? Thus making the two cases above behave the same and like the first case (but with a much simpler implementation). Or should this just always list the directory and the file can be seen when viewing that directory (so making both cases like the second case -- and also a much simpler implementation).

Or should we leave this complicated and confusing behavior that is not documented anywhere.

Note that I also don't like the =library-ignore, =library-combine-up, and =library-no-combine stuff (which is also not documented anywhere that I know of), but that behavior is all preserved and apparently there are people that use these things.

It's hard to follow the complicated and confusing behavior, but my feeling is that if there is a folder tree:

foo/
    something.pg
    bar/
        (possibly zero) files that do not end in .pg
        files that start with . regardless of what they end with
        baz/
            something.pg

then the only folders you should be presented with are foo and foo/bar/baz. I don't understand why if the pg file name matches the folder name, that would matter.

I know of one case of someone using =library-ignore, and they only started a few months ago. They have a (locally) shared library that includes a folder with test pg files and did not want that folder appearing for general faculty. I'm sure they could find a different solution, like move that test folder somewhere else. I would not object to removing those weird things.

I do find these in the OPL though:

webwork-open-problem-library/OpenProblemLibrary/Union/setMVderivatives/gradient-3/=library-combine-up
webwork-open-problem-library/OpenProblemLibrary/Union/setMVvectors/an12_3_25/=library-combine-up
webwork-open-problem-library/OpenProblemLibrary/Union/setMVlevelsets/levels-5/=library-combine-up
webwork-open-problem-library/OpenProblemLibrary/Union/setMVlevelsets/levels-7/=library-combine-up

It's hard to follow the complicated and confusing behavior, but my feeling is that if there is a folder tree:
foo/
    something.pg
    bar/
        (possibly zero) files that do not end in .pg
        files that start with . regardless of what they end with
        baz/
            something.pg
then the only folders you should be presented with are foo and foo/bar/baz. I don't understand why if the pg file name matches the folder name, that would matter.

That is how it is with this pull request. However, with the develop branch all of foo, foo/bar, and foo/bar/baz are presented. Basically with this pull request, hidden files are completely ignored. The Mojo::File::list method does not return those unless you tell it to by adding the hidden => 1 option (and I didn't add that).

My question restated with a folder tree is:

If you have

foo/
    foo.pg
    bar.png

then the behavior (both with develop and this pull request) is to not list foo in the directories, but if you view the [templates folder] the file foo/foo.pg is shown.

If you have

foo/
    foo.pg

then with the develop branch the foo directory is not listed and there is no way to view foo/foo.pg at all. With this pull request foo is listed with the directories, and you can view foo/foo.pg with the directory.

My question then is basically, should we make the above two cases behave the same, and if so which way (like the first or like the second)?

openwebwork / webwork2