Split up drasil-gool/Drasil into GOOL, GProc, and Shared folders.
Contributes to #3934
Splitting up drasil-gool/Drasil into GOOL, GProc, and Shared folders.
Looks like a good start!
I noticed, it looks like you left everything in the LanguageRenderer folder under Shared. That doesn't seem right to me, since some of these files are (or should be, in some cases) OO-only or Proc-only.
- The Java, C#, C++, Python, and swift renderers are all OO-only, and the Julia renderer is Proc-only
-
AbstractProc.hsis Proc-only -
CLike.hscan likely stay here; as its title suggests it has to do with "similarity to C" rather than "OO vs Procedural" -
CommonPseudoOO.hsshould be OO-only based on the name, but I wouldn't be surprised if the Julia renderer uses it. Maybe take a look at what functionality it provides and whether it's worth moving toGOOLor leaving inShared. - From a quick glance at the others, I believe they are all shared as well.
There might be a good reason you didn't move those files I mentioned; it just seems to me that moving them (and taking a look at CommonPseudoOO.hs) would make more organizational sense.
I didnt remove the helpers from the CommonOO file just in case ( which is causing the HLint error ). If the new file is fine Ill clean it.
Also, plenty of import errors, Im working on them
A (now deleted / edited) comment said:
Do you think it is a good idea to split the CommonPseudoOO file into CommonOO, CommonDocs, CommonTypes.. ? This way I can more the OO specific ones to GOOL
That does seem like a good idea. to me.
Also: I'll do a full review once the CI passes.
After taking a better look at CommonPseudoOO, it definitely seems feasible to split it up in some way. It's split into commented sections that state which renderers use each one, which should make it easier. If nothing better works (though I like your idea to split it up based on Docs, OO, Types, etc), you could move everything used by Julia into a new file (e.g. CommonShared or something), and move the rest of the original file into the GOOL folder.
Concerning CommonPseudoOO.hs, I think it should remain in the Shared folder. The Julia renderer (and others) rely on several of its helper functions.
I got this error that I couldnt fix. It keeps showing despite the fact that I included it in Drasil.Shared.InterfaceCommon
Please push your partial work (that gives the above error) to a branch onto the repo. That way I can go into the details and see what the problem is.
I didn't notice it was missing, probably due to eye strain from too much screen time.
Even after fixing one error, hundreds more appear, a lesson in patience.
I guess misplacements caused the errors. I am trying to correct that.
How is it coming along?
This is perhaps also a lesson in doing refactors in small pieces instead of trying to jump to the desired end-state all at once. [If you're stuck, it might even be worth starting from scratch but with all your experience, and redo it in small pieces.]
I have successfully separated the common functions from CommonPseudoOO.hs used by Julia into the Common.hs file.
Regarding issue https://github.com/JacquesCarette/Drasil/issues/3934#issue-2478660676:
-
drasil-goolshould be renamed todrasil-codegen(per Dr. Carette's suggestion to reflect GOOL and GProc collectively). -
drasil-gool/Drasilhas been split into GOOL, GProc, and Shared folders. - Common functions from
CommonPseudoOO.hshave been moved toCommon.hs. - Awaiting feedback on suggestion at https://github.com/JacquesCarette/Drasil/issues/3934#issuecomment-2913925980 (related to issue https://github.com/JacquesCarette/Drasil/issues/4083).
Oh? Why was the PR closed? Was that an accident?
I came here to ask the same question!
This PR looks very close to ready! @sarrasoussia Can you just confirm if this PR closes that issue or if it only contributes to it? Would there be any work left to address that issue? It looks like you've addressed everything to me, but I want to double-check 😄
Also, @JacquesCarette has some comments that you should address regarding imports. Now that I've merged main into this branch as well, you might see the CI fail because of unused dependencies, name shadowing and such that you will need to address.
@balacij I just need to review the imports and write the ones used explicitly
Why did we need to create the variable classVar then assign it to extClassVar ? Isnt it more appropriate to just write extClassVar = CP.classVar R.classVar. It is creating an ambiguous occurrence of ‘classVar’ error.
Instances in Haskell don't work that way. The left hand side (LHS) are in the context of defining methods of OOVariableSym and the rhs from the current context. So classVar = CP.classVar R.classVar is not creating a variable, it is assigning to a (required) method.