FileData, FileAndContents, PackageFiles

[B-rando1]

Today I noticed something interesting.

Drasil/code/drasil-code/lib/Language/Drasil/Code/FileData.hs

Line 9 in a415dcc

data FileAndContents = FileAndContents {filePath :: FilePath, fileDoc :: Doc}

• FileAndContents is a path and a doc

---

Drasil/code/drasil-gool/lib/Drasil/Shared/AST.hs

Line 59 in a415dcc

data FileData = FileD {filePath :: FilePath, fileMod :: ModData}

• FileData is a path and a ModData

• Drasil/code/drasil-gool/lib/Drasil/Shared/AST.hs

  Line 75 in a415dcc

  data ModData = MD {name :: String, modDoc :: Doc}

  • ModData is a name and a doc
• So FileData is a subtype of FileAndContents, modulo some isomorphism

---

Drasil/code/drasil-code/lib/Language/Drasil/Code/PackageFiles.hs

Line 21 in a415dcc

newtype PackageFiles = PackageFiles [(FilePath, Doc)]

• PackageFiles is a list of pairs of path and doc
• So essentially a list of FileAndContents, modulo some isomorphism

---

I feel like at least one of these types is unnecessary! From an in-person chat with @JacquesCarette, PackageFiles is on the chopping block. I think it still makes sense to keep FileAndContents as the type containing the subset of FileData that drasil-code needs.

Beyond that, consolidatePackageFiles is doing some silly things and can be removed or at least heavily reduced. Most of the other functions in that file are likely unnecessary as well.

[JacquesCarette]

I think it makes sense to have FileAndContents (and to use it instead of tuples).

One question: is the filePath a mere path in FileAndContents, i.e. to a directory, or is it a full path to an actual file? Same question for FileData. And what is the relation of the name in ModData to its path?

[B-rando1]

Those are good questions. We should make Drasil has a consistent answer.

• From looking around, it looks like the FilePath in FileAndContents is always used to store the full path to a file, not just a directory.
  • The one wrinkle in this is Main.hs's createCodeFile function, where it first creates and enters packageName before doing anything else. This essentially makes the pathname packageName ++ filePath for each file created there.
  • I think this is done earlier in the pipeline for the rest of GOOL, but it's honestly hard to tell.
• It's not extremely clear what the relationship between the name in ModData to the filePath in FileData. My current best guess comes from fileDoc, which is called by drasil-code. It suggests that the module name is simply its path minus the extension.

It's not immediately clear where these names actually come from, since they get passed through so many functions and representations before they're used for the filepath. I can dig deeper when I have more time if it's important.

[JacquesCarette]

It might make sense to separate path from file name (and indeed, file name + extension) in our 'raw data'.

We need to better understand this to come up with a reasonable design.

[B-rando1]

One thing I didn't get to in #4721 - #4721 (comment)

Probably not a bad thing that we didn't let that block the PR; it's more of an extension anyway. I'll take another look after we finish up #4722.

[B-rando1]

In my previous comment, I mentioned a discussion where we looked at this line:

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

Line 133 in 092f539

hasPathAndDocToFileAndContents (progMods $ packageProg $ unReprPack pckg)

It's a bit messy because of how nested the [FileData] that we want to map over is. I had talked about making a HasCodeFiles class with instances letting us get the [FileData] out more cleanly in places like this.

I tried it out and it is possible, but I'm not sure if we gain enough with that level of polymorphism. We can improve readability in this function by using pattern-matching within a let-binding. I don't know if we do enough converting between different types that hold a list of code files to warrant having a typeclass for them. I'll open a PR where I clean it up a bit without adding that typeclass, and we can discuss it some more there.