Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

[[Bug 18305]] Add Synonyms to Dictionary #5669

Merged
merged 3 commits into from Jul 6, 2017

Conversation

bwmilby
Copy link
Contributor

@bwmilby bwmilby commented Jul 4, 2017

LC 7 allowed searching for synonyms within the dictionary. Implementation appears to have been by duplicating the dictionary entries for each synonym. This fix will duplicate the dictionary entry for each synonym when the entries are built from the repository. "Synonym of" is added to the display name of each non-glossary entry (glossary synonyms do not seem to mean that the terms mean the same thing - i.e. card button, card field, card control, card object).

The change is made in the "revDocsParseDictionaryToLibraryArray" handler. On each pass through the loop, the resulting array is tested to see if the "Synonyms" key contains data. If it does, then loop through each synonym and duplicate the entry (taking into account that glossary entries specify themselves as a synonym). The display name is changed to the synonym and "Synonym of ..." is added as a cross-reference for non-glossary entries.

LC 7 allowed searching for synonyms within the dictionary.  Implementation appears to have been by duplicating the dictionary entries for each synonym.  This fix will duplicate the dictionary entry for each synonym when the entries are built from the repository.  "Synonym of" is added to the display name of each non-glossary entry (glossary synonyms do not seem to mean that the terms mean the same thing - i.e. card button, card field, card control, card object).

The change is made in the "revDocsParseDictionaryToLibraryArray" handler.  On each pass through the loop, the resulting array is tested to see if the "Synonyms" key contains data.  If it does, then loop through each synonym and duplicate the entry (taking into account that glossary entries specify themselves as a synonym).  The display name is changed to the synonym and "Synonym of ..." is added as a cross-reference for non-glossary entries.
@livecodeali
Copy link
Member

Hi @bwmilby, thanks very much for the fix and very thorough commit message! It looks good to me. Regarding your second point, yes it perhaps does need something like what you mention there - although I'm slightly wary of something called display syntax being essentially made unfit for display (I know it wouldn't currently matter as only the first element of the array is displayed, but this might change at some point).

I think the best option would be to make the dictionary search use display name as well as display syntax in its search - along with this commit that would solve the problem.

Update the bug fix to add the synonym to the "display syntax" element.  Each dictionary entry only carries the first "syntax" entry over to the "display syntax" as [1].  Using [2] for the synonym entry (it does not get displayed in the dictionary window).
@bwmilby
Copy link
Contributor Author

bwmilby commented Jul 6, 2017

Just saw your comment after I committed a change with the 'display syntax' addition. Not sure how to back that commit out though.

@mwieder
Copy link
Contributor

mwieder commented Jul 6, 2017

@bwmilby I wouldn't back out the commit, just change it locally and push again.
If you do need to back it out, then git reset locally and force push the change.

@bwmilby
Copy link
Contributor Author

bwmilby commented Jul 6, 2017

Pull #1658 created on livecode-ide to add the display name to the search in the IDE.

Back out the "display syntax" addition to the dictionary.  Pull request livecode#1658 created on the IDE to update the dictionary search to include the "display name".
@livecodeali
Copy link
Member

@livecode-vulcan review ok 941f789

@livecodeali
Copy link
Member

@bwmilby We can squash these commits when merging so that it comes out as a single commit anyway. Thanks very much!

@livecode-vulcan
Copy link
Contributor

💙 review by @livecodeali ok 941f789

@livecode-vulcan
Copy link
Contributor

😎 test success 941f789

  • try-community-armv6-android-api8: success
  • try-community-armv6-android-api9: success
  • try-community-js-emscripten-sdk1.35: success
  • try-community-universal-ios-iphoneos10.3: success
  • try-community-universal-ios-iphonesimulator10.3: success
  • try-community-universal-mac-macosx10.6: success
  • try-community-universal-mac-macosx10.9: success
  • try-community-x86-linux-debian7: success
  • try-community-x86-linux-debian8: success
  • try-community-x86_64-linux-debian7: success
  • try-community-x86_64-linux-debian8: success
  • try-community-x86-win32: success
  • try-community-x86_64-win32: success

@livecodeali livecodeali merged commit d72f163 into livecode:develop Jul 6, 2017
@livecodeali livecodeali added this to the 9.0.0-dp-8 milestone Jul 6, 2017
@bwmilby bwmilby deleted the bugfix-18305 branch July 6, 2017 21:34
bwmilby added a commit to bwmilby/livecode that referenced this pull request Sep 24, 2017
This reverts commit d72f163.
Due to a regression where search terms such as "mobile" will display the "iphone" synonyms first and give the impression that those are the preferred entries.  Changes are being made to the IDE (dictionary viewer) to implement the synonym search there.

This code also caused issues with duplicates being displayed in the new Autocomplete feature.
livecode-vulcan added a commit that referenced this pull request Sep 25, 2017
Revert "[[Bug 18305]] Add Synonyms to Dictionary (#5669)"

This reverts commit d72f163.
Due to a regression where search terms such as "mobile" will display the "iphone" synonyms first and give the impression that those are the preferred entries.  Changes are being made to the IDE (dictionary viewer) to implement the synonym search there.

This code also caused issues with duplicates being displayed in the new Autocomplete feature.
livecodepanos added a commit that referenced this pull request Sep 25, 2017
Revert "[[Bug 18305]] Add Synonyms to Dictionary (#5669)"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants