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

Give better error message if a helpfile is missing #939

Merged
merged 1 commit into from
Nov 9, 2016

Conversation

fingolfin
Copy link
Member

A recent xgap release had a bug: it's old-style manual was missing a file (gobject.tex) in the release tarball (see gap-packages/xgap#3). This lead to a rather unhelpful error message by the GAP help system:

Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `PositionStream' on 1 arguments called from
PositionStream( stream 
 ) at /mnt/disk2/hudson-slave/workspace/GAP-update-release-test/GAPCOPTS/64bui\
ld/GAPGMP/gmp/GAPTARGET/standard/label/64bit/gap4r8/lib/helpdef.gi:306 called \
from
...

That was the error @alex-konovalov got, I actually saw a different one:

Error, Function Calls: <func> must return a value in
  return HELP_PRINT_SECTION_TEXT( info, chnr, secnr ); at /home/r/mhorn/gap/lib/helpdef.gi:669 called from
HELP_BOOK_HANDLER.(book.handler).HelpData( book, entrynr, type ) at /home/r/mhorn/gap/lib/helpbase.gi:783 called from
...

This PR improves on the situation, by inserting explicit error messages that at least give a hint as to what went wrong, making it easier to debug similar issues in the future.

I also fixed HELP_PRINT_SECTION_TEXT to always return a value (see also issue #464 and PR #466).

... or if it inaccessible (e.g. due to missing read access)
@fingolfin fingolfin added the kind: bug Issues describing general bugs, and PRs fixing them label Nov 6, 2016
@codecov-io
Copy link

Current coverage is 48.82% (diff: 0.00%)

Merging #939 into master will decrease coverage by <.01%

@@             master       #939   diff @@
==========================================
  Files           424        424          
  Lines        222089     222094     +5   
  Methods        3426       3426          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits         108441     108436     -5   
- Misses       113648     113658    +10   
  Partials          0          0          

Powered by Codecov. Last update becaeff...5a526a4

@ChrisJefferson
Copy link
Contributor

Both changes here look good (and unfortunately tricky to test, so lack of tests is OK I think).

@olexandr-konovalov
Copy link
Member

The different error messages are fine: I have reported what I have seen in the stable-4.8 branch while testing the release candidate for the next minor release, while tests that were performed in the master branch failed in the same way as @fingolfin observed:

########> Diff in /home/hudson/hudson/workspace/GAP-pkg-update-master-quicktes\
t/GAPCOPTS/64build/GAPGMP/gmp/GAPTARGET/standard/label/graupius/GAP-pkg-update\
-master-snapshot/tst/teststandard/helpsys.tst:24
# Input is:
for i in [1..Length(HELP_LAST.TOPICS)] do HELP(String(i)); od;
# Expected output:
# But found:
Error, Function Calls: <func> must return a value
########

The PR looks good to me, and it addresses both issues. I will merge it after GAP 4.8.6 will be out, to not to interfere with the release workflow.

@fingolfin
Copy link
Member Author

@alex-konovalov This PR is against master, not stable. I did not think it was worth the bother. So I don't see how merging this could interfere with the release workflow... ?

BTW, this is one of those PRs where I'd suggest to rebase, not merge (which the github API nicely allows these days).

@olexandr-konovalov
Copy link
Member

@fingolfin ah, you're right - this is master branch of course. I will rebase it now - thanks.

@olexandr-konovalov olexandr-konovalov merged commit a624385 into gap-system:master Nov 9, 2016
@fingolfin fingolfin deleted the mh/help branch November 9, 2016 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants