JacquesCarette
`getAcc` versus `short`. What is the difference?
While investigating #4008 I ran into an interesting issue. When creating the SRS documents we often use abbreviations/acronyms, however we seem to get these in at least two different ways. First, we use getAcc which gets an "abbreviation in 'Sentence' form from a 'CI'." https://github.com/JacquesCarette/Drasil/blob/196370e5a3ca95d9be69f9155f6c664dda897f5a/code/drasil-lang/lib/Language/Drasil/Chunk/CommonIdea.hs#L51-L53 It does this through the abrv function. However, we also have short which uses the Ch constructor for a Sentence, and uses the ShortStyle style which matches in spec to run lookupS. https://github.com/JacquesCarette/Drasil/blob/196370e5a3ca95d9be69f9155f6c664dda897f5a/code/drasil-lang/lib/Language/Drasil/Development/Sentence.hs#L22-L24 https://github.com/JacquesCarette/Drasil/blob/196370e5a3ca95d9be69f9155f6c664dda897f5a/code/drasil-lang/lib/Language/Drasil/Sentence.hs#L100-L101 lookupS "Look[s] up the acronym/abbreviation of a term. Otherwise returns the singular form of a term. Takes a chunk database and a 'UID' associated with the term." https://github.com/JacquesCarette/Drasil/blob/196370e5a3ca95d9be69f9155f6c664dda897f5a/code/drasil-printers/lib/Language/Drasil/Printing/Import/Helpers.hs#L65-L68 To me it seems that they are very similar but only just because they accomplish seemingly the same thing, which I understand does not make them necessarily identical. I am wondering what the difference is (if any). The reason I ask this is because of my implementation for #4008 (see #4080) had an interesting bug where it would only work on some acronyms, and this seemed to be the root cause. Using getAcc does not create a Sentence that has ShortStyles associated with it, which is what I am using to add the tooltips.
I also did a bit of searching of past issues and it seems that getAcc is not the only function that may be affecting things, as similar ones such as getA or getAccStr also exist. I will need to do more investigation on these, however.
https://github.com/JacquesCarette/Drasil/issues/3472 is a relevant issue -- did you already have a look at it? It would be good to also gather some information on how short is used.
Aside note: I'm not a fan of short being called short when it also defaults to the long-form if a short-form isn't available. But I'm also not sure of a better name...
I remember taking a quick look at #3472; reading it more carefully it seems like getAcc can be replaced by short entirely. I tried it and the stable changes seemed to be only related to adding the tooltips like it should. I haven't touched getA, however.
For short I was thinking it could be renamed to something like smartShort, smartA, shortSmart, or even just smart. Just some ideas I had right now, they may all be unusable.
What is the difference between getA (which is what lookupS ends up using) and getAcc? It does seem like two wildly different paths to get at (essentially) the same thing, but with difference levels of smarts.
getA definitely seems like the more appropriate option for getting an acronym/abbreviation, as it allows for CommonIdeas to not require an abbreviation. Using short results in lookupS, which is smart and uses the UID to create an acronym if none exists. Since getA returns a Maybe String, while getAcc returns a Sentence, all instances of getAcc can be replaced by short. Furthermore, getA is not used in the same way getAcc is. getAcc is used in many different places when building Sentences, while getA's only usage (in terms of outputting an acronym) is in lookupS, which handles scenarios where no acronym is given. Completely ruling out getAcc in favor of short (and consequently, getA) is definitely possible, and should be done in my opinion. I already did this in #4080 (specifically b4ed833), so the required changes can be found there.
"uses the UID" had me confused (using the String of a UID is a big Drasil taboo), but you really meant that it uses the ChunkDB to look things up.
I'll take a look at the details of #4080 then.
Since #4080 was merged, I think we can close this? Is there any documentation updates in the code we can make to clear this up for future folks @jackwyand ?