SuggestedSpellings extended and renamed to HELP_SEARCH_ALTERNATIVES #1144

olexandr-konovalov · 2017-02-15T01:13:08Z

This addresses #1019.

Please make sure that this pull request:

is submitted to the correct branch (the stable branch is only for bugfixes)
contains an accurate description of changes for the release notes below
provides new tests or relies on existing ones
correctly refers to other issues and related pull requests

Tick all what applies to this pull request

Adds new features
Improves and extends functionality
Fixes bugs that could lead to crashes
Fixes bugs that could lead to incorrect results
Fixes bugs that could lead to break loops

Write below the description of changes (for the release notes)

The search for the documentation of system setters and testers
such as e.g. ?SetIsMapping and ?HasIsMapping will return
corresponding attributes and properties. e.g. IsMapping.

codecov · 2017-02-15T01:19:39Z

Codecov Report

Merging #1144 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1144      +/-   ##
==========================================
+ Coverage   59.21%   59.25%   +0.04%     
==========================================
  Files         434      434              
  Lines      230814   230825      +11     
==========================================
+ Hits       136682   136786     +104     
+ Misses      94132    94039      -93

Impacted Files	Coverage Δ
lib/helpbase.gi	`41.74% <100%> (+12.42%)`	✅
lib/grppc.gi	`61.39% <ø> (-0.17%)`	❌
lib/tietze.gi	`41.99% <ø> (+0.13%)`	✅
lib/grpfp.gi	`61.29% <ø> (+0.19%)`	✅
lib/stbcrand.gi	`90.46% <ø> (+0.29%)`	✅
lib/matint.gi	`68.34% <ø> (+0.37%)`	✅
lib/wordass.gi	`39.65% <ø> (+0.52%)`	✅
lib/helpdef.gi	`38.16% <ø> (+0.66%)`	✅
lib/liefam.gi	`40.79% <ø> (+0.79%)`	✅
lib/kernel.g	`61.9% <ø> (+6.34%)`	✅
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 436cdb0...06ffefa. Read the comment docs.

olexandr-konovalov · 2017-02-15T10:07:47Z

For any search term starting with Has or Set, this will search for HasSomething, SetSomething and Something. This is a slight overhead which I believe may be neglected. Of course one could decide to search only for Something plus the original search term (just in case it does not mean setter/tester, like e.g. in SetupCache or HastaLaVista - in this case it will also search for HasupCache and upCache etc. - this is also neglectible).

olexandr-konovalov · 2017-02-16T14:50:59Z

This should be ready to merge now. I've updated this PR to achieve target goals for test coverage. That revealed that one of the tests, (formerly from helpsys.tst, now helptools.tst) became broken because of set stabiliser. I've added a check that the next character after set or has is not a space.

fingolfin

Looks good to me (modulo typos) and worked in a brief test.

fingolfin · 2017-02-18T14:07:40Z

lib/helpbase.gi

+
+  # Now chop the string 'topic' into a list of lists, each of them either
+  # a list of all variants from the respective spelling pattern or just
+  # a one-element list with the "glueing" string between two pattersn or


typo: pattersn -> patterns

fingolfin · 2017-02-18T14:11:50Z

tst/teststandard/helptools.tst

@@ -0,0 +1,17 @@
+# This test checks various auxiliary functions used by the help system.
+#
+# For the test that systematically checks each mahual section, see


typo: mahual -> manual

fingolfin · 2017-02-18T14:12:23Z

tst/teststandard/helptools.tst

+gap> HELP_SEARCH_ALTERNATIVES("hasismapping");
+[ "hasismapping", "ismapping", "setismapping" ]
+gap> HELP_SEARCH_ALTERNATIVES("setismapping");
+[ "hasismapping", "ismapping", "setismapping" ]


perhaps also include HELP_SEARCH_ALTERNATIVES("ismapping") ?

fingolfin

Actually, I just found one caveat: Try ?hash. Without this PR, it lists four (relevant) help entries for me). Without it, it turns this into a search for "h" and thus finds countless entries.

Similar for sets, SetX, ...

So at the very least, I guess it should require the search string to have more than 4-5 letters.

Better would be if it only took the alternates into account if there are no its. I.e. if "topic" has a hit, just use it, if there are no hits, remove the has resp. set prefix. Of course such a change is more complicated than what is in this PR :-/

codecov-io · 2017-02-18T17:29:23Z

Codecov Report

Merging #1144 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1144      +/-   ##
==========================================
+ Coverage   59.39%   59.43%   +0.03%     
==========================================
  Files         434      434              
  Lines      230498   230509      +11     
==========================================
+ Hits       136913   137004      +91     
+ Misses      93585    93505      -80

Impacted Files	Coverage Δ
lib/helpbase.gi	`41.74% <100%> (+12.42%)`	✅
src/system.c	`62.84% <ø> (-1.05%)`	❌
lib/tietze.gi	`41.99% <ø> (-0.6%)`	❌
lib/straight.gi	`25.28% <ø> (-0.42%)`	❌
lib/stbcrand.gi	`90.46% <ø> (+0.09%)`	✅
lib/grpfp.gi	`61.29% <ø> (+0.19%)`	✅
lib/helpdef.gi	`38.16% <ø> (+0.66%)`	✅
lib/grppcint.gi	`99.32% <ø> (+0.67%)`	✅
lib/polyrat.gi	`59.46% <ø> (+0.75%)`	✅
lib/csetperm.gi	`91.35% <ø> (+3.7%)`	✅
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7f2b93...a58f136. Read the comment docs.

olexandr-konovalov · 2017-02-18T18:13:36Z

@fingolfin I've made an update. Good catch about hash and SetX, thanks!

fingolfin · 2017-02-18T19:54:53Z

lib/helpbase.gi

+
+for topic in topics do
+  if Length(topic) > 4 and topic{[1..3]} in [ "has" , "set" ] and topic[4]<>' ' then
+    shorttopic := topic{[4..Length(topic)]};


I think there should be some comments here. One that explains what's going on ("ensure HasIsMapping is found in help even if only IsMapping ismdocumentd"), another which explains that x > 4 is not an off-by-one accident, but rather intentionally (and also what the intention is). Otherwise I am afraid somebody might "fix" it someday .

Now it also ensures that the search for system setters and testers such as e.g. ?SetIsMapping and ?HasIsMapping will return corresponding attributes and properties. e.g. IsMapping.

olexandr-konovalov · 2017-02-19T19:31:28Z

@fingolfin comments added, good suggestion.

olexandr-konovalov force-pushed the help-setter-tester branch from 2e1fb99 to de5888f Compare February 15, 2017 01:19

olexandr-konovalov force-pushed the help-setter-tester branch from de5888f to 6c8a3e7 Compare February 15, 2017 01:20

olexandr-konovalov added the topic: documentation Issues and PRs related to documentation label Feb 15, 2017

olexandr-konovalov added this to the GAP 4.9.0 milestone Feb 16, 2017

olexandr-konovalov force-pushed the help-setter-tester branch 2 times, most recently from 43aa919 to 06ffefa Compare February 16, 2017 12:24

olexandr-konovalov mentioned this pull request Feb 18, 2017

Remove template for GitHub pull requests #1154

Merged

9 tasks

fingolfin approved these changes Feb 18, 2017

View reviewed changes

fingolfin requested changes Feb 18, 2017

View reviewed changes

olexandr-konovalov force-pushed the help-setter-tester branch from 06ffefa to 54eaf30 Compare February 18, 2017 17:29

olexandr-konovalov force-pushed the help-setter-tester branch from 54eaf30 to 155fb10 Compare February 18, 2017 17:31

fingolfin reviewed Feb 18, 2017

View reviewed changes

SuggestedSpellings extended and renamed to HELP_SEARCH_ALTERNATIVES

a58f136

Now it also ensures that the search for system setters and testers such as e.g. ?SetIsMapping and ?HasIsMapping will return corresponding attributes and properties. e.g. IsMapping.

olexandr-konovalov force-pushed the help-setter-tester branch from 155fb10 to a58f136 Compare February 19, 2017 14:47

fingolfin approved these changes Feb 21, 2017

View reviewed changes

fingolfin merged commit 86321d9 into gap-system:master Feb 21, 2017

fingolfin mentioned this pull request Sep 7, 2017

Release notes for GAP 4.9 #1699

Closed

olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 20, 2018

olexandr-konovalov mentioned this pull request Aug 19, 2019

Document function SetConjugacyClasses #3607

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SuggestedSpellings extended and renamed to HELP_SEARCH_ALTERNATIVES #1144

SuggestedSpellings extended and renamed to HELP_SEARCH_ALTERNATIVES #1144

olexandr-konovalov commented Feb 15, 2017 •

edited

Loading

codecov bot commented Feb 15, 2017 •

edited

Loading

olexandr-konovalov commented Feb 15, 2017 •

edited

Loading

olexandr-konovalov commented Feb 16, 2017

fingolfin left a comment

fingolfin Feb 18, 2017

fingolfin Feb 18, 2017

fingolfin Feb 18, 2017

fingolfin left a comment

codecov-io commented Feb 18, 2017 •

edited

Loading

olexandr-konovalov commented Feb 18, 2017

fingolfin Feb 18, 2017

olexandr-konovalov commented Feb 19, 2017

SuggestedSpellings extended and renamed to HELP_SEARCH_ALTERNATIVES #1144

SuggestedSpellings extended and renamed to HELP_SEARCH_ALTERNATIVES #1144

Conversation

olexandr-konovalov commented Feb 15, 2017 • edited Loading

Tick all what applies to this pull request

Write below the description of changes (for the release notes)

codecov bot commented Feb 15, 2017 • edited Loading

Codecov Report

olexandr-konovalov commented Feb 15, 2017 • edited Loading

olexandr-konovalov commented Feb 16, 2017

fingolfin left a comment

Choose a reason for hiding this comment

fingolfin Feb 18, 2017

Choose a reason for hiding this comment

fingolfin Feb 18, 2017

Choose a reason for hiding this comment

fingolfin Feb 18, 2017

Choose a reason for hiding this comment

fingolfin left a comment

Choose a reason for hiding this comment

codecov-io commented Feb 18, 2017 • edited Loading

Codecov Report

olexandr-konovalov commented Feb 18, 2017

fingolfin Feb 18, 2017

Choose a reason for hiding this comment

olexandr-konovalov commented Feb 19, 2017

olexandr-konovalov commented Feb 15, 2017 •

edited

Loading

codecov bot commented Feb 15, 2017 •

edited

Loading

olexandr-konovalov commented Feb 15, 2017 •

edited

Loading

codecov-io commented Feb 18, 2017 •

edited

Loading