Deprecate undocumented extra arguments for DeclareGlobalFunction and InstallGlobalFunction
#3694
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The extra argument was never documented for `InstallGlobalFunction`. In contrast, for `DeclareGlobalFunction`, the documentation for a long time claimed that it required exactly (!) two arguments, although all examples only showed one argument, and the documentation never said what the other argument was supposed to do. Various people then made differing guesses as to the supposed meaning of the extra arguments: some put in an info string; some put in a list of input filters; some just a single filter. This PR removes several of these from the GAP library itself. Various packages still use these extra arguments, and loading them will trigger the warnings. Some already fixed it in their repositories and just need to make releases. When the time comes for releasing GAP 4.12, we can see if all packages are adapted, and then depending on that either turn the warning into an error; or conversely, increase the warning level to 2, to hide it from regular users who might be confused by the message.
fingolfin
added
topic: library
release notes: to be added
fingolfin
force-pushed
the
mh/globalfunc-args
branch
from
aa6cc30
to
748c74b
Compare
ChrisJefferson
approved these changes
Oct 7, 2019
wilfwilson
approved these changes
Oct 7, 2019
|
@fingolfin for most packages this should only break their tests, but there is a case when it broke the bugfix test for the core GAP system when GAP is loaded with |
|
? Fixing that requires no crisp release, just some trivial modifications to that tst file to tweak info levels |
olexandr-konovalov
pushed a commit
to olexandr-konovalov/gap
that referenced
this pull request
Nov 6, 2019
After gap-system#3694, a warning `DeclareGlobalFunction: too many arguments` is shown when Crisp is loaded.
|
Submitted #3726 to silence the warning in the bugfix test. |
olexandr-konovalov
pushed a commit
to olexandr-konovalov/gap
that referenced
this pull request
Nov 7, 2019
After gap-system#3694, a warning `DeclareGlobalFunction: too many arguments` is shown when Crisp is loaded.
olexandr-konovalov
pushed a commit
that referenced
this pull request
Nov 7, 2019
After #3694, a warning `DeclareGlobalFunction: too many arguments` is shown when Crisp is loaded.
PaulaHaehndel
added
release notes: added
fingolfin
added
the
kind: removal or deprecation
fingolfin
changed the title
DeclareGlobalFunction and InstallGlobalFunction
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The extra argument was never documented for
InstallGlobalFunction. Incontrast, for
DeclareGlobalFunction, the documentation for a long timeclaimed that it required exactly (!) two arguments, although all
examples only showed one argument, and the documentation never said what
the other argument was supposed to do. Various people then made
differing guesses as to the supposed meaning of the extra arguments:
some put in an info string; some put in a list of input filters; some
just a single filter. This PR removes several of these from the GAP
library itself.
Various packages still use these extra arguments, and loading them will
trigger the warnings. Some already fixed it in their repositories and
just need to make releases. When the time comes for releasing GAP 4.12,
we can see if all packages are adapted, and then depending on that
either turn the warning into an error; or conversely, increase the
warning level to 2, to hide it from regular users who might be confused
by the message.