Add options to debug package manager

[fingolfin]

Motivated by #928.

[fingolfin]

Of course this should also be documented; I'll do that once we settled on what final form this should have.

[fingolfin]

I wonder if it might not be better to separate debug and this "verbose" mode: leaving around partial packages could be a stumbling block for non-experts. Yet we may end up asking these very people to use debug if they report issues installing packages. Of course we can ask them to GAP.Packages.remove("blah") afterwards, but still...

So one idea would be to also add verbose::Bool = false as another kwarg. Of course then one can wonder what happens if both quiet and verbose are set (an error?). Seeing how quiet is off by default, perhaps a better alternative would be to add a "verbosity level". So:

• verbose::Int = 0 is a new kwarg and directly corresponds to the InfoLevel
  • alternative names: verbosity, verbosity_level, infolevel, info_level, ... ?
  • default value is 0 to match the current default of quiet=true
• we either drop quiet; or keep it as "deprecated" ("please instead use ...")
  • if set to true that sets verbose to 0, if set to false it sets it to whatever is the default value for InfoPackageManager (or whatever value we prefer)
  • implementation: `function foo(quiet::Bool = true, verbose::Int = (quiet ? 0 : 2))
• thus if both are given, verbose wins (we could of course also use nothing to detect the case and report an error, but I feel this is overthinking it... on the long run I'd just remove quiet)
• we remove the option of "not changing the infolevel (it is redundant if the user has full control over the info level anyway)

[ThomasBreuer]

My viewpoint was that PackageManager's default info level is reasonable (otherwise this level should be changed there) but that there are situations where we want to suppress all info messages, for example if a package installation is triggered by the request to load a package. This is why quiet arose instead of access to the various info levels (which are documented in PackageManager).

There is of course no problem with the viewpoint that one should be able to set the info level in each single function call. I do not see why this is helpful here, but it is surely a "conceptual" feature which is currently not supported by PackageManager

(Note that by default, the functions from GAP.Packages except load do show the info messages at the current level --the default for the keyword argument quiet is false for them. Just for load, quiet = true is the default, and the value is handed over to install when load was called with the install = true option.)

Debugging is something different, the current proposal for debug changes the behaviour of the functions in the sense that RemovePackage calls are deliberately omitted.
I think that setting debug = true in the call of a function should imply setting the info level to the highest meaningful value, perhaps except if the info level is explicitly set in the same function call.

[fingolfin]

Of course we can remove this hack once we are using a PackageManager release with gap-packages/PackageManager#119 or something equivalent in it

[ThomasBreuer]

The analogous function ReplaceGapFunc in Oscar.jl/gap/OscarInterface/gap/OscarInterface.gi creates a global variable Concatenation("_ORIG_", name) if it does not yet exist. Perhaps do this also here?

[ThomasBreuer]

I have made just two comments, one concrete about the code and one about the conceptual questions.
Yes, if setting the info level per function call is supported then an additional quiet option is not reasonable.
A function call that requests diagnostic output but without using debug mode would then have a keyword argument info_level = 4 or so.

The current default makes sense:
Show no messages if GAP.Packages.load is called, but use the current info level of PackageManager for the other functions of GAP.Packages.

[ThomasBreuer]

My viewpoint was that PackageManager's default info level is reasonable (otherwise this level should be changed there) but that there are situations where we want to suppress all info messages, for example if a package installation is triggered by the request to load a package. This is why quiet arose instead of access to the various info levels (which are documented in PackageManager).

There is of course no problem with the viewpoint that one should be able to set the info level in each single function call. I do not see why this is helpful here, but it is surely a "conceptual" feature which is currently not supported by PackageManager

(Note that by default, the functions from GAP.Packages except load do show the info messages at the current level --the default for the keyword argument quiet is false for them. Just for load, quiet = true is the default, and the value is handed over to install when load was called with the install = true option.)

Debugging is something different, the current proposal for debug changes the behaviour of the functions in the sense that RemovePackage calls are deliberately omitted.
I think that setting debug = true in the call of a function should imply setting the info level to the highest meaningful value, perhaps except if the info level is explicitly set in the same function call.

[fingolfin]

I fixed a few issues with this.

Overall I agree with your analysis @ThomasBreuer and since you've approved it, I assume we can merge it after CI runs (but since we'll meet in half an hour anyway, we can also talk about it then)

[codecov]

Codecov Report

Merging #931 (2a7ab1c) into master (1ad0308) will increase coverage by 0.05%.
The diff coverage is 58.82%.

@@            Coverage Diff             @@
##           master     #931      +/-   ##
==========================================
+ Coverage   75.72%   75.78%   +0.05%     
==========================================
  Files          51       51              
  Lines        4161     4162       +1     
==========================================
+ Hits         3151     3154       +3     
+ Misses       1010     1008       -2     

Files Changed	Coverage
src/wrappers.jl	ø
src/packages.jl	58.82%