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

Extra hint in "No Method Found" if one of the arguments is 'fail' #460

Merged
merged 1 commit into from
Feb 8, 2016

Conversation

olexandr-konovalov
Copy link
Member

This PR is an alternative to PR #355 by @ChrisJefferson.

It originates from issue #354 initiated by @cdwensley, and adds an extra hint in the "No Method Found" message in the case where one of the arguments is fail, as suggested by @stevelinton.

For example:

gap> Read(Filename(DirectoriesLibrary("tst"),"testall.g"));
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
The 1st argument is 'fail' which might point to an earlier problem
Error, no 1st choice method found for `Read' on 1 arguments called from
...

@olexandr-konovalov olexandr-konovalov added the gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 label Jan 14, 2016
@fingolfin fingolfin changed the title Extra hint in "No Method Found" if of the arguments is 'fail' Extra hint in "No Method Found" if one of the arguments is 'fail' Jan 15, 2016
@olexandr-konovalov
Copy link
Member Author

Can we get this reviewed an merged? I could then cherry-pick it onto stable-4.8.

@olexandr-konovalov
Copy link
Member Author

@stevelinton @fingolfin does the new error message

gap> Read(Filename(DirectoriesLibrary("tst"),"testall.g"));
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
The 1st argument is 'fail' which might point to an earlier problem
Error, no 1st choice method found for `Read' on 1 arguments called from
...

look OK to you? In this case, this is ready to be merged.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.8.2 milestone Feb 1, 2016
@fingolfin
Copy link
Member

I am a bit torn on this one. On the one hand, it seems strange to have a special case for this particular thing. On the other hand, it seems like a pragmatic solution to a common problem.

Hmm, thinking about this, I can sum it up like this: My ratio tells me this is not pure, and thus "bad", my guts tell me "screw purity, Alex put work into this, it helps with a real world problem, it hurts nobody, and if somebody has a better idea, they can provide their own PR with that, replacing this solution".

So, I'll just merge it for now.

fingolfin added a commit that referenced this pull request Feb 8, 2016
Extra hint in "No Method Found" if one of the arguments is 'fail'
@fingolfin fingolfin merged commit 97fa819 into gap-system:master Feb 8, 2016
@olexandr-konovalov olexandr-konovalov deleted the fail-method-arg branch February 8, 2016 20:32
@olexandr-konovalov olexandr-konovalov added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes labels Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapsagedays2016 Issues and PRs that arose at https://www.gapdays.de/gap-sage-days2016 kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants