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

Module structure refactorings #80

Merged
merged 15 commits into from
Feb 11, 2025
Merged

Conversation

ibbem
Copy link
Collaborator

@ibbem ibbem commented Feb 10, 2025

This fixes some inconsistencies in the code, splits the language properties into separate files (similar to how the Agda standard library does it) and tries to make it easier to use.

Copy link
Member

@pmbittner pmbittner left a comment

Choose a reason for hiding this comment

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

Thank you very much for these welcome changes! This looks a lot better now. I went through the commits chronologically one by one. So sometimes a comment got invalidated or addressed by a later change.

I don't have many requests, just a few questions and, as always, I would like to see a little bit of documentation here and there. :)

(Also: Nobody will think that I contributed anything to this code base after you acquired every line of git blame. 🤣 )

src/Vatras/Lang/All.agda Outdated Show resolved Hide resolved
src/Vatras/Lang/All.agda Show resolved Hide resolved
src/Vatras/Translation/Lang/2CC-to-ADT.agda Outdated Show resolved Hide resolved
src/Vatras/Translation/Lang/ADT-to-2CC.agda Outdated Show resolved Hide resolved
src/Vatras/Translation/Lang/VariantList-to-CCC.lagda.md Outdated Show resolved Hide resolved
src/Vatras/Lang/All/Fixed.agda Show resolved Hide resolved
src/Vatras/Translation/Lang/2CC-to-ADT.agda Show resolved Hide resolved
@pmbittner
Copy link
Member

Do we have a version number? If so, we should probably increment?

@ibbem ibbem force-pushed the module-structure-refactorings branch from 96a9be0 to 2be45ac Compare February 10, 2025 18:28
ibbem added 15 commits February 10, 2025 19:35
All other languages just append an "L" not a "VL" suffix.
It's way easier to make module parameters implicit for some definitions
and not others than to add a module parameter after the fact.
Although `renaming` makes the identifiers shorter (especially the
semantics), the suffixes where context dependent (e.g., ₐ, ₙ for ADT,
NADT and ₂, ₙ for 2CC, NCC) and used inconsistently.

Note that The length issue is only really an issue for `VariantList`
which could be named `CaO` as in the paper to make it shorter.
This order is already used in a lot of places and makes the selection
language the first argument which is nice when fixing the selection
language.
This order is already used in a lot of places and makes the selection
language the first argument which is nice when fixing the selection
language.
Importing `Vatras.Lang.All` or `Vatras.Lang.All.Fixed` should be the
canonical form for importing any language.
I'm not sure when agda-mode started to generate these files, or if this
is a bug. Anyways, these can be safely ignored.
@ibbem ibbem force-pushed the module-structure-refactorings branch from 2be45ac to ec4b3f4 Compare February 10, 2025 18:35
@ibbem
Copy link
Collaborator Author

ibbem commented Feb 10, 2025

Yes, there is one version number in default.nix. As far as I can tell that's the only place.

@ibbem ibbem changed the base branch from main to develop February 10, 2025 18:38
@ibbem
Copy link
Collaborator Author

ibbem commented Feb 10, 2025

Does this adequately fix #76?

@pmbittner
Copy link
Member

Not yet but it is a first necessary step towards fixing #76. To address #76 fully, it would also be good to have modules like Vatras.Lang.All.Fixed but again fixed to strings or natural (Vatras.Lang.All.OnString, Vatras.Lang.All.OnNat?). In these modules, 𝔽, 𝕍, and 𝔸 would be fixed to the respective values as exemplified in the description of #76. So these would be convenience modules to write examples fast and easy. And I guess Nat and String are the most requested atoms. I am open to other suggestions. The most annoying part in #76 was specifying the atom set.

I think we could do all this in another PR though.

@pmbittner pmbittner merged commit 5993340 into develop Feb 11, 2025
1 check passed
@pmbittner pmbittner mentioned this pull request Feb 11, 2025
3 tasks
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