Skip to content

Split drasil printers#4836

Open
JoeZZG wants to merge 4 commits intomainfrom
refactor/split-drasil-printers
Open

Split drasil printers#4836
JoeZZG wants to merge 4 commits intomainfrom
refactor/split-drasil-printers

Conversation

@JoeZZG
Copy link
Copy Markdown
Collaborator

@JoeZZG JoeZZG commented Mar 24, 2026

#4831
drasil-printing is a new package, which contains the shared printing infrasturecture:
Printing.AST, Printing.LayoutObj, Printing.PrintingInformation, Printing.Import.* (all expression/document converters), Printing.Citation, Printing.Helpers, Plain.Print, Config

drasil-printers is now smaller and only contains format-specific printers (HTML, TeX, JSON, Markdown and Debug), and depends on drasil-printing

No other things changed

@balacij
Copy link
Copy Markdown
Collaborator

balacij commented Mar 24, 2026

Restarted the CI because it was hanging on the system requirements.

Comment on lines +24 to +54
- drasil-database
- drasil-lang
- drasil-metadata
- drasil-theory
- drasil-utils
- split

ghc-options:
- -Wall
- -Wredundant-constraints

library:
source-dirs: lib
exposed-modules:
- Language.Drasil.Config
- Language.Drasil.Plain.Print
- Language.Drasil.Printing.AST
- Language.Drasil.Printing.Citation
- Language.Drasil.Printing.Helpers
- Language.Drasil.Printing.Import
- Language.Drasil.Printing.Import.CodeExpr
- Language.Drasil.Printing.Import.Document
- Language.Drasil.Printing.Import.Expr
- Language.Drasil.Printing.Import.Helpers
- Language.Drasil.Printing.Import.Literal
- Language.Drasil.Printing.Import.ModelExpr
- Language.Drasil.Printing.Import.Sentence
- Language.Drasil.Printing.Import.Space
- Language.Drasil.Printing.Import.Symbol
- Language.Drasil.Printing.LayoutObj
- Language.Drasil.Printing.PrintingInformation
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should drasil-printing be doing this much? I thought that these modules were meant to be kept in drasil-printers?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What I think this does, in part, is to separate the parts that are about specific output formats (kept in drasil-printers) from "the rest". So that drasil-printing becomes the 'awfully messy' package and drasil-printers may be regarded as fairly clear.

Of course, the new drasil-printing should not have so many exposed modules! It would be best to see how few of these modules need to be exposed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, ok. In that case, should LayoutObj, PrintingInformation, and Language.Drasil.Printing.AST stay in drasil-printers?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Once @JoeZZG trims down the exposed modules, we'll see. I guess one could see now, by staring at the dependency graph of the unsplit version.

Comment on lines +24 to +54
- drasil-database
- drasil-lang
- drasil-metadata
- drasil-theory
- drasil-utils
- split

ghc-options:
- -Wall
- -Wredundant-constraints

library:
source-dirs: lib
exposed-modules:
- Language.Drasil.Config
- Language.Drasil.Plain.Print
- Language.Drasil.Printing.AST
- Language.Drasil.Printing.Citation
- Language.Drasil.Printing.Helpers
- Language.Drasil.Printing.Import
- Language.Drasil.Printing.Import.CodeExpr
- Language.Drasil.Printing.Import.Document
- Language.Drasil.Printing.Import.Expr
- Language.Drasil.Printing.Import.Helpers
- Language.Drasil.Printing.Import.Literal
- Language.Drasil.Printing.Import.ModelExpr
- Language.Drasil.Printing.Import.Sentence
- Language.Drasil.Printing.Import.Space
- Language.Drasil.Printing.Import.Symbol
- Language.Drasil.Printing.LayoutObj
- Language.Drasil.Printing.PrintingInformation
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What I think this does, in part, is to separate the parts that are about specific output formats (kept in drasil-printers) from "the rest". So that drasil-printing becomes the 'awfully messy' package and drasil-printers may be regarded as fairly clear.

Of course, the new drasil-printing should not have so many exposed modules! It would be best to see how few of these modules need to be exposed.

@JoeZZG JoeZZG requested a review from JacquesCarette March 27, 2026 21:28
Copy link
Copy Markdown
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.

Better. But now we need to know why the others must be exposed.

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.

3 participants