Implement Proposal #3948: Enhance README Generation #3950

Xinlu-Y · 2024-09-03T17:01:50Z

This PR implements the design outlined in proposal #3948. The main objective is to ensure that the scope and motivation are included in both the SystemInformation and the generated README files.

Key Updates:

Optional Subsections: Motivation an Purpose are now optional subsections under introInfo.
'What' Section: background and scope are now optional subsections under whatInfo.

Closes: #3948, #3159, #3248, #3971

- Restructure `introInfo` to focus on `motivation` and `purpose`. - Adjust `whatInfo` to address `background` and `scope`.

Resolved GHC warnings by fixing name shadowing issues and removing redundant bracket. Added new fields into sglPend.

JacquesCarette

Your actual changes are actually fine - but you are doing them against a bad design... so the right thing to do is to first fix the design, then apply these changes again.

Now, if you want, I would be fine with accepting the code as-is and have you fix the design after. I suspect that that is actually more work!

JacquesCarette · 2024-09-05T02:24:21Z

code/drasil-code/lib/Language/Drasil/Code/Imperative/GOOL/ClassInterface.hs

@@ -52,6 +52,10 @@ type CaseName = String
 type ExamplePurpose = String
 -- | Description of example
 type ExampleDescr = String
+-- | Motivation of example


While it is good that you are following the established pattern in this code, the problem is that the pattern is wrong... in that GOOL shouldn't know anything at all about 'examples'.

The real problem isn't these types, but ReadMeInfo which shouldn't be defined inside GOOL at all!

So I think the first step of the implementation, as a separate PR, would be to first fix this and move this information out of GOOL.

JacquesCarette · 2024-09-05T02:26:34Z

code/drasil-code/lib/Language/Drasil/Code/Imperative/Generator.hs

@@ -150,6 +150,10 @@ genPackage unRepr = do
 (foldlSent $ purpose $ codeSpec g)
 bckgrnd = show $ sentenceDoc db Implementation OneLine
 (foldlSent $ background $ codeSpec g)
+ mtvtn = show $ sentenceDoc db Implementation OneLine


In that same refactor, I think all the stuff that concerns README generation should be moved out of this file. It could go into Language/Drasil/Code/Imperative/Generator/README.

JacquesCarette · 2024-09-05T02:28:42Z

code/drasil-code/lib/Language/Drasil/CodeSpec.hs

@@ -47,6 +47,10 @@ data CodeSpec where
 purpose :: Purpose,
 -- | Example Background.
 background :: Background,
+ -- | Example Scope.


I don't think this information should be in CodeSpec either. It is fine to have it in SystemInformation though.

Remove unneed comments.

Xinlu-Y · 2024-09-09T18:25:57Z

I got stuck after representing the fields purpose, background, motivation, and scope as ConceptInstances. By doing this, these pieces of information are stored in the sysinfodb, which is then copied into CodeSpec. However, in the genPackage function, I need assistance with retrieving a ConceptInstance by its UID. Any guidance from those familiar with this process would be appreciated.

JacquesCarette · 2024-09-09T20:22:54Z

If @balacij can't help you tonight, then I will be at McMaster tomorrow and should be able to help (likely time: early afternoon).

- Used the generated `sysInfo` lens to access nested fields in `SystemInformation` from `CodeSpec`.

Fix warnings.

JacquesCarette · 2024-09-13T19:24:50Z

code/drasil-code/lib/Language/Drasil/Code/Imperative/Descriptions.hs

@@ -40,7 +41,7 @@ unmodularDesc = do
 implTypeStr Program = "program"
 implTypeStr Library = "library"
 return $ show $ sentenceDoc (sysinfodb spec) Implementation OneLine $ capSent $
- foldlSent ([S "a", S (implTypeStr (implType g)), S "to"] ++ purpose spec)
+ foldlSent ([S "a", S (implTypeStr (implType g)), S "to"] ++ codeSpec g ^. sysInfo .purpose)


You should not need to "double lens" (i.e. .sysInfo .purpose). If you make the right instance, .purpose should be enough.

JacquesCarette · 2024-09-13T19:25:20Z

code/drasil-code/lib/Language/Drasil/Code/Imperative/GOOL/ClassInterface.hs

@@ -41,36 +40,3 @@ class AuxiliarySym r where

 auxHelperDoc :: r (AuxHelper r) -> Doc
 auxFromData :: FilePath -> Doc -> r (Auxiliary r)
-


Oh, this is a nice clean-up!

JacquesCarette · 2024-09-13T19:26:33Z

code/drasil-code/lib/Language/Drasil/Code/Imperative/GOOL/LanguageRenderer/PythonRenderer.hs

-import Language.Drasil.Code.Imperative.GOOL.ClassInterface (ReadMeInfo(..),PackageSym(..),
- AuxiliarySym(..))
+import Language.Drasil.Code.Imperative.GOOL.ClassInterface (PackageSym(..), AuxiliarySym(..))
+import Language.Drasil.Code.Imperative.ReadMe.Import (ReadMeInfo(..))


Can you explain to me why all the language renderers need access to ReadMeInfo?

JacquesCarette · 2024-09-13T19:28:13Z

code/drasil-code/lib/Language/Drasil/Code/Imperative/WriteReadMe.hs

@@ -5,7 +5,7 @@ module Language.Drasil.Code.Imperative.WriteReadMe (
 import Language.Drasil.Choices (ImplementationType(..))
 import Language.Drasil.Printers (makeMd, introInfo, verInfo, unsupOS,
 extLibSec, instDoc, endNote, whatInfo)
-import Language.Drasil.Code.Imperative.GOOL.ClassInterface (ReadMeInfo(..))
+import Language.Drasil.Code.Imperative.ReadMe.Import (ReadMeInfo(..))


So much better!

JacquesCarette · 2024-09-13T19:29:34Z

code/drasil-code/lib/Language/Drasil/CodeSpec.hs

@@ -70,7 +67,9 @@ data CodeSpec where
 -- automatically define.
 mods :: [Mod], -- medium hack
 -- | The database of all chunks used in the problem.
- sysinfodb :: ChunkDB
+ sysinfodb :: ChunkDB,


I thought the aim was to rename this CodeSpec to OldCodeSpec and then make CodeSpec has just 2 fields?

JacquesCarette · 2024-09-23T20:22:56Z

@Xinlu-Y can you please fix the conflict that was recently created?

- Extracted parts of the original `CodeSpec` into a new` OldCodeSpec` data type. - Used `makeClassyFor` to create accessor functions such as `pNameO`, `authorsO`, `inputsO`, etc., which are now used to access corresponding fields in `OldCodeSpec`. - Generated the `oldCodeSpec` lens, which allows easy access and modification of `OldCodeSpec` fields from a `CodeSpec` instance. - Updated `CodeSpec` to hold both `SystemInformation` and a reference to `OldCodeSpec`. - Modified related functions (e.g., `codeSpec`, `oldcodeSpec`) .

- Introduced `makeClassy` to generate the `HasSystemInformation` type class and associated lenses for the `SystemInformation` data structure. - Updated `CodeSpec` to use the `HasSystemInformation` type class.

Xinlu-Y · 2024-10-02T19:24:38Z

@JacquesCarette Dr. Carette, Could you please review the latest changes when you have time? I have addressed the earlier feedback and resolved the conflicts. Thank you!

Xinlu-Y added 8 commits August 27, 2024 16:30

added scope and motivation fields into SI

89886ef

linked SI to README

a849147

added new fields into example SI

0974d17

Stabilized projectile READMEs

2de0bd8

Refactor documentation sections:

915505d

- Restructure `introInfo` to focus on `motivation` and `purpose`. - Adjust `whatInfo` to address `background` and `scope`.

added new fields into example SI

267b547

Remove redundant period.

57e1713

Updated stable READMEs.

0609bdf

Xinlu-Y requested review from JacquesCarette, smiths, samm82, bmaclach and cd155 as code owners September 3, 2024 17:01

Removed redundant bracket.

3de170f

Resolved GHC warnings by fixing name shadowing issues and removing redundant bracket. Added new fields into sglPend.

Xinlu-Y force-pushed the expandREADME branch from 84a79fc to 3de170f Compare September 3, 2024 18:16

JacquesCarette requested changes Sep 5, 2024

View reviewed changes

Xinlu-Y requested a review from balacij as a code owner September 5, 2024 20:36

Refactor: Move README-specific information out of ClassInterface.

b82cba4

Remove unneed comments.

Xinlu-Y force-pushed the expandREADME branch from c576027 to b82cba4 Compare September 5, 2024 20:59

Xinlu-Y added 3 commits September 10, 2024 17:48

remove fields.

8b0141e

Refactored CodeSpec to reference SystemInformation.

f803135

- Used the generated `sysInfo` lens to access nested fields in `SystemInformation` from `CodeSpec`.

Replaced direct field access with sysInfo lens in CodeSpec.

085e146

Fix warnings.

Xinlu-Y force-pushed the expandREADME branch from 31a4181 to 085e146 Compare September 11, 2024 12:22

JacquesCarette requested changes Sep 13, 2024

View reviewed changes

Xinlu-Y added 2 commits September 23, 2024 16:57

Merge branch 'main' into expandREADME

b8e6715

stabilized

b9488c2

Xinlu-Y mentioned this pull request Sep 27, 2024

Manually Defining Lens with Type Constraints #3971

Open

Xinlu-Y added 3 commits September 27, 2024 16:32

Refactored SystemInformation with classy lenses

6e7045b

- Introduced `makeClassy` to generate the `HasSystemInformation` type class and associated lenses for the `SystemInformation` data structure. - Updated `CodeSpec` to use the `HasSystemInformation` type class.

Refactored access to sysinfodb using lens in CodeSpec

10ae63b

JacquesCarette approved these changes Oct 2, 2024

View reviewed changes

JacquesCarette merged commit 8490ac2 into main Oct 2, 2024
5 checks passed

balacij deleted the expandREADME branch October 3, 2024 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Proposal #3948: Enhance README Generation #3950

Implement Proposal #3948: Enhance README Generation #3950

Xinlu-Y commented Sep 3, 2024 •

edited

Loading

JacquesCarette left a comment

JacquesCarette Sep 5, 2024

JacquesCarette Sep 5, 2024

JacquesCarette Sep 5, 2024

Xinlu-Y commented Sep 9, 2024

JacquesCarette commented Sep 9, 2024

JacquesCarette Sep 13, 2024

JacquesCarette Sep 13, 2024

JacquesCarette Sep 13, 2024

JacquesCarette Sep 13, 2024

JacquesCarette Sep 13, 2024

JacquesCarette commented Sep 23, 2024

Xinlu-Y commented Oct 2, 2024

		@@ -41,36 +40,3 @@ class AuxiliarySym r where

		auxHelperDoc :: r (AuxHelper r) -> Doc
		auxFromData :: FilePath -> Doc -> r (Auxiliary r)

Implement Proposal #3948: Enhance README Generation #3950

Implement Proposal #3948: Enhance README Generation #3950

Conversation

Xinlu-Y commented Sep 3, 2024 • edited Loading

JacquesCarette left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xinlu-Y commented Sep 9, 2024

JacquesCarette commented Sep 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacquesCarette commented Sep 23, 2024

Xinlu-Y commented Oct 2, 2024

Xinlu-Y commented Sep 3, 2024 •

edited

Loading