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

Implement HasUID class to LabelledContents and Section #1071

Merged
merged 20 commits into from
Dec 3, 2018
Merged

Conversation

halonazhao
Copy link
Collaborator

In this PR:

  1. LabelledContent and Section were implemented HasUID class. Since they both have label field and label has a UID, the HasUID directly pulls the UID from the label to attach to the Chunk.
  2. UID field was added to Reference2 for future use.

Question:
Since there were some labels created for list, do we remove them to keep them as unlabelled contents? @JacquesCarette

@@ -172,7 +172,7 @@ dataConstraintParagraph hasUncertainty tableRef middleSent trailingSent = mkPara
-- makes a list of references to tables takes
-- l list of layout objects that can be referenced
-- outputs a sentence containing references to the layout objects
listofTablesToRefs :: (HasShortName l, Referable l) => [l] -> Sentence
listofTablesToRefs :: (HasUID l, HasShortName l, Referable l) => [l] -> Sentence
Copy link
Owner

Choose a reason for hiding this comment

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

I think this HasUID constraint can be lifted now.

@@ -45,7 +45,7 @@ roc_temp_simp_deriv_sentences = map foldlSentCol [
QT.temp vol [makeRef2S newA3, makeRef2S newA4, makeRef2S newA5],
genDefDesc5 density mass vol]

genDefDesc1 :: (HasShortName x, Referable x) => x -> UnitalChunk -> [Sentence]
genDefDesc1 :: (HasUID x, HasShortName x, Referable x) => x -> UnitalChunk -> [Sentence]
Copy link
Owner

Choose a reason for hiding this comment

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

And this one too? Though we might consider making HasUID a pre-condition of Referable.

@@ -344,7 +344,7 @@ phys_sys_desc_p1 = physSystIntro slope how intrslce slice
(S "slice base") fig_indexconv
where how = S "as a series of" +:+ phrase slice +:+. plural element

physSystIntro :: (NamedIdea a, NamedIdea b, NamedIdea c, HasShortName d, Referable d) =>
physSystIntro :: (NamedIdea a, NamedIdea b, NamedIdea c, HasUID d, HasShortName d, Referable d) =>
Copy link
Owner

Choose a reason for hiding this comment

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

Same here.


makeCiteS :: Citation -> Sentence
makeCiteS = Ref2 . makeCite

-- | Create a reference for a URI
makeURI :: String -> ShortName -> Reference2
makeURI ra sn = Reference2 URI ra sn
makeURI ra sn = Reference2 "fixme" URI ra sn
Copy link
Owner

Choose a reason for hiding this comment

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

I don't really want to accept a PR with a "fixme" like that in it...

Copy link
Owner

@JacquesCarette JacquesCarette left a comment

Choose a reason for hiding this comment

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

Only the one thing needs adjusted, then I can merge this in.

@@ -434,7 +437,10 @@ goal_statements_list' = map (foldlSent) [goal_statements_G_linear,
goal_statements_G_angular, goal_statements_G_detectCollision,
goal_statements_G_collision]

goal_statements_list = enumSimple 1 (getAcc goalStmt) goal_statements_list'
goal_label :: Label
goal_label = mkLabelRALst "goalGM" "goalGM"
Copy link
Owner

Choose a reason for hiding this comment

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

It seems that goal_label (and solution_label and ...) are duplicated for each example. They should be pulled out into drasil-docLang.

@JacquesCarette
Copy link
Owner

If you could do this change soon, that would be nice, so that I can indeed merge this in. There is some code I'd like to do [I've caught at least one case of a makeRef2 being called on a Label], but it makes more sense to do that 'on top' of this rather than independently.

@halonazhao
Copy link
Collaborator Author

Sorry I didn't put the priority on this because I really wanted to push on my process so that some fixes wouldn't kill my patience. Does putting labels in docLang is enough? Or even take the constructor to docLang? @JacquesCarette

@JacquesCarette JacquesCarette merged commit 7f54389 into master Dec 3, 2018
@JacquesCarette
Copy link
Owner

Branch merged - should I also delete it, because the work on this (addUID) is 'done'?

@halonazhao
Copy link
Collaborator Author

Yes, please. We are done with addUID branch.

@JacquesCarette JacquesCarette deleted the addUID branch December 3, 2018 17:50
@JacquesCarette
Copy link
Owner

Done.

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.

None yet

2 participants