-
Notifications
You must be signed in to change notification settings - Fork 22
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
show GAP help in Julia with 'GAP.show_GAP_help' and via ?
#251
Conversation
Codecov Report
@@ Coverage Diff @@
## master #251 +/- ##
==========================================
- Coverage 70.21% 69.18% -1.04%
==========================================
Files 51 54 +3
Lines 3280 3605 +325
==========================================
+ Hits 2303 2494 +191
- Misses 977 1111 +134
|
I updated this PR and added some functionality to display the GAP help when the Julia help mode is called on |
?
@ThomasBreuer @mohamed-barakat @fingolfin Please have a look. I am going to rebase this PR once all the file-reordering is done. |
julia> GAP.show_GAP_help("HomalgMatrix") works. In the notebook it looks like this: https://gist.github.com/mohamed-barakat/f0335ef22e4a8618dd195cca0b6ac9b4 |
@mohamed-barakat Thanks for testing this feature. |
I think even in the notebook it looks kinda reasonable. |
@sebasguts The first lines of help output in the notebook are broken into lines in a strange way. |
Well, just when you use that printing function. If you use |
@ThomasBreuer: Actually I also like to have this "inline" option for presentations. |
@sebasguts: The updated link https://gist.github.com/mohamed-barakat/f0335ef22e4a8618dd195cca0b6ac9b4 shows that also ?HomalgMatrix works. |
Also this works fine for me. |
One last comment. I get colors in the Jupyter notebook which are not visible in the gist link. |
@mohamed-barakat which colors do you mean? The ones in the ouput of |
@sebasguts: yes, although the output of |
Indeed. But you can see them on nbviewer |
Could you please resolve the conflicts in this PR? I would like to merge it. |
@mohamed-barakat we'll resolve them once PR #249 is merged, not before. Also, I'd like to review this PR once, too |
@@ -0,0 +1,536 @@ | |||
############################################################################# | |||
## | |||
## copied from GAP's 'lib/helpbase.gi', then adjusted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that's pretty close of course. If lib/helpbase.gi
is ever changed, we'd have to track that, too. I wonder if in the future, we might work on refactoring the code on the GAP side so that we can use it directly, instead of replicating it here (e.g. the code Thomas wrote here could be put into GAP, and then lib/helpbase.gi
rewritten to use it).
@sebasguts: Excellent. |
@mohamed-barakat I am going to wait untill #249 is merged, because otherwise I would resolve conflicts twice. |
I will move the code over into a separate file. I would guess the comments about the GAP code are optional @fingolfin? Because I really do not want to wait with this until we have refactored the GAP help system. |
I addressed the comments and updated this PR |
I rebased the PR |
@sebasguts the GAP refactoring remarks as "not even optional": they were ramblings about the distant future. Don't worry about them here :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebase this again to resolve a recent conflict, and removed pkg/GAPJulia/JuliaInterface/julia/libgap.jl
(which was accidentally added back). Assuming tests pass now, I think this is OK to merge.
OK, looking at coverage made me realize that we don't have any tests for this. Could we please add some? Possible both on the GAP and the Julia side? |
@fingolfin The only tests I can think of are tests whether calling the functions in question return some strings, regardless of the contents. If this is sufficient then I can add such tests. |
5fe2080
to
b7d6921
Compare
I think that the reported code coverage is wrong. For example, the function (I have already tried to rebase the pull request, what else should I do?) |
@ThomasBreuer the coverage data looks correct to me. I don't see a direct call to |
@fingolfin thanks for your hint. In this situation, requests for help just reply that nothing reasonable was found, (The current code can run into errors when the manuals are missing/incomplete, |
Well of course we could compiled the GAP manual as well. Probably a good idea anyway, as users will want to have access to it. Well, or we switch to using GAP 4.10.2, which of course includes prebuilt manuals. |
@ThomasBreuer on |
As an experiment, I cherry-picked commit 6466e3d onto this PR, to see if it has any effect on the coverage. |
This is a first approximation. The functions in the file `helpstring.g` arose from those in GAP's library file `helpbase.gi`. The idea is to extract the requested information from the text format files of the GAP manuals, and to collect it in a string, also in the case where several matches are found. Currently the following problems are known. - When the last subsection of a section is requested then also the part of the next section (in the same chapter) until the start of its first subsection is shown; apparently the sections themselves have no entry numbers in the sense of `HELP_BOOK_HANDLER.(book.handler).HelpData`. (An example is `SizeScreen`.) - Some pieces of GAP help are very long, an example is the entry for `Size` in the HAP package. On the other hand, duplicate entries are filtered out, which should perhaps also done in GAP's help system.
The tests cover the situation where one asks for help about a topic, and where an exact match is found or just partial matches are found or no matches are found at all. There is code in `helpstring.g` which cannot be reached via these requests, such as requests for the next or previous topic in a manual (which do not make sense in the current setup for Julia), but I wanted to have a complete "string returning" variant of GAP's `helpbase.gi`.
- fixed the computation of the end lines of subsections - changed input and output format of `HELP_String` - added more tests
- If a `.txt` file is missing then the `HelpData` result has no component `start`. - Some entries in `book.entries` store strings in their first position, not numbers; be aware of this when navigating to the next entry. - If there is exactly one exact match then do not suppress partial matches. (This does not happen inside GAP because partial matches are joined with the exact matches; but then one cannot restrict the search to exact matches.)
This is a first approximation.
The functions in the file
helpstring.g
arose from thosein GAP's library file
helpbase.gi
.The idea is to extract the requested information
from the text format files of the GAP manuals,
and to collect it in a string,
also in the case where several matches are found.
Currently the following problems are known.
When the last subsection of a section is requested
then also the part of the next section (in the same chapter)
until the start of its first subsection is shown;
apparently the sections themselves have no entry numbers
in the sense of
HELP_BOOK_HANDLER.(book.handler).HelpData
.(An example is
SizeScreen
.)Some pieces of GAP help are very long,
an example is the entry for
Size
in the HAP package.On the other hand, duplicate entries are filtered out,
which should perhaps also done in GAP's help system.