Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: factor out text handling as new modules Text and Loctext #159

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

favonia
Copy link
Contributor

@favonia favonia commented Oct 7, 2024

The current Diagnostic module bundles diagnostics and text operations together. The planned lower-level debugging interface #138 could reuse text operations but not (high-level) diagnostics. This PR factors out text operations as new modules Text and Loctext so that the debugging interface does not have to depend on the Diagnostic module.

Here are all the bindings being moved:

Old binding New binding
Diagnostic.text (type) Text.t
Diagnostic.text (function) Text.make
Diagnostic.textf Text.makef
Diagnostic.ktextf Text.kmakef
Diagnostic.loctext (type) Loctext.t
Diagnostic.loctext (function) Loctext.make
Diagnostic.loctextf Loctext.makef
Diagnostic.kloctextf Loctext.kmakef
Diagnostic.string_of_text Text.to_string

@favonia favonia marked this pull request as draft October 7, 2024 22:50
@favonia favonia requested review from jonsterling and mmcqd and removed request for jonsterling and mmcqd October 7, 2024 23:59
@favonia favonia changed the title refactor: factor out text handling as a new module refactor: factor out text handling as new module Text Oct 8, 2024
@favonia favonia force-pushed the text branch 2 times, most recently from 6484093 to 6462e8e Compare October 8, 2024 11:14
@favonia favonia changed the title refactor: factor out text handling as new module Text refactor: factor out text handling as new modules Text and Loctext Oct 8, 2024
@favonia favonia force-pushed the text branch 2 times, most recently from ddb0aa7 to 7e2dde4 Compare October 8, 2024 11:23
Copy link
Collaborator

@mikeshulman mikeshulman left a comment

Choose a reason for hiding this comment

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

Seems reasonable, looking in from the outside.

@favonia favonia marked this pull request as ready for review October 9, 2024 14:08
@favonia favonia merged commit d38ae1a into main Oct 9, 2024
1 check passed
@favonia favonia deleted the text branch October 9, 2024 14:08
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.

2 participants