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

Add GetCaptionOrHeadword() to CmPicture #288

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Add GetCaptionOrHeadword() to CmPicture #288

merged 2 commits into from
Nov 14, 2023

Conversation

mark-sil
Copy link
Contributor

@mark-sil mark-sil commented Nov 7, 2023

This change is to add a “Caption or Headword” field in Fieldworks
Dictionary Configuration. The feature is described in the first part
of this Jira ticket:
https://jira.sil.org/browse/LT-21648


This change is Reviewable

This change is to add a “Caption or Headword” field in Fieldworks
Dictionary Configuration. The feature is described in the first
part of this Jira ticket:
https://jira.sil.org/browse/LT-21648
@jasonleenaylor
Copy link
Contributor

src/SIL.LCModel/DomainImpl/CmPicture.cs line 254 at r1 (raw file):

				{
					// Need to call this to get the actual ws.
					WritingSystemServices.GetMagicStringAlt(Cache, wsId, Hvo, Caption.Flid, true, out wsActual);

Shouldn't this be the flid for the Headword?

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @mark-sil)

This change is to add a “Caption or Headword” field in Fieldworks
Dictionary Configuration.
https://jira.sil.org/browse/LT-21648
Copy link
Contributor Author

@mark-sil mark-sil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor)


src/SIL.LCModel/DomainImpl/CmPicture.cs line 254 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Shouldn't this be the flid for the Headword?

Changed flid to zero and use a GetMagicStringAlt() wrapper class that more accurately describes why we were making the call.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mark-sil)

@mark-sil mark-sil merged commit 5a74dbb into master Nov 14, 2023
3 checks passed
@mark-sil mark-sil deleted the feature/LT-21648 branch November 14, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants