Skip to content

feat(domain_debugging): add a pretty output option for domain of expressions#1624

Merged
ozgurakgun merged 19 commits intomainfrom
testing-new-domain-output-format
Mar 13, 2026
Merged

feat(domain_debugging): add a pretty output option for domain of expressions#1624
ozgurakgun merged 19 commits intomainfrom
testing-new-domain-output-format

Conversation

@ca1umac
Copy link
Contributor

@ca1umac ca1umac commented Mar 11, 2026

Description

Adds expr_domains as an option to the --output-format= argument in the pretty subcommand.
Prints out a JSON of expressions, detailing:

  1. a nice format of what the expression is (e.g., a string of the essence syntax)
  2. the domain of that expression
  3. the children of that expression

Related issues

helping toward #1547 by adding the data source for testing.

Key changes

Adding a new output format

How to test/review

Run cargo run pretty <file> --output-format=expr_domains and manually check that the result is as expected.

@conjure-bot
Copy link

conjure-bot bot commented Mar 11, 2026

Documentation Coverage

Report: https://conjure-cp.github.io/conjure-oxide-reports/pr/1624/coverage-docs/index.html

This PR: 43.48% documented, 4.97% with examples (8/70/161)
Main: 43.48% documented, 4.97% with examples (8/70/161)
Delta: docs +0.00 pp, examples +0.00 pp

@conjure-bot
Copy link

conjure-bot bot commented Mar 11, 2026

Code Coverage

Report: https://conjure-cp.github.io/conjure-oxide-reports/pr/1624/coverage-code/index.html
Diff report: https://conjure-cp.github.io/conjure-oxide-reports/pr/1624/coverage-code/diff-coverage.html

This PR: 74.63% lines (16739/22428)
Diff: 67.00% lines (23/34 measured; 239 changed)
Main: 72.21% lines (16711/23143)
Delta vs main: +2.43 pp

@ozgurakgun
Copy link
Contributor

I like the CLI UI for this! Nitpick: can it be called expression-domains instead of expr_domains please?

@ca1umac ca1umac self-assigned this Mar 12, 2026
@ca1umac ca1umac marked this pull request as ready for review March 12, 2026 00:56
@ca1umac
Copy link
Contributor Author

ca1umac commented Mar 12, 2026

@ozgurakgun I think this is ready to go. I think we may as well keep the nested structure since it keeps more information than not. I think it'd be more helpful for debugging if nothing else.

@ca1umac ca1umac requested a review from ozgurakgun March 12, 2026 20:13
Copy link
Contributor

Choose a reason for hiding this comment

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

how do you like this? I kept yours as expression-domains-json but I am tempted to keep just this.

Copy link
Contributor

Choose a reason for hiding this comment

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

to be clear indentation indicates nesting

Copy link
Contributor

Choose a reason for hiding this comment

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

and :: is meant to mean domain-of. like how : is type-of.

@ozgurakgun
Copy link
Contributor

btw

find x,y,a,b : set (maxSize 2) of int(1..3)
such that x union y subsetEq a intersect b

prints

((x union y) subsetEq (a intersect b)) :: bool
 (x union y) :: set  of set  of int(1..3)
  x :: set (maxSize(2)) of int(1..3)
  y :: set (maxSize(2)) of int(1..3)
 (a intersect b) :: set  of set  of int(1..3)
  a :: set (maxSize(2)) of int(1..3)
  b :: set (maxSize(2)) of int(1..3)

which has several issues in it.

@ca1umac
Copy link
Contributor Author

ca1umac commented Mar 13, 2026

I think yours is significantly more readable, it's brilliant. I don't see any reason to keep the json one, it'll just be confusing.

I've gotten rid of the superfluous brackets in the attributes, I'm assuming that was one of the issues. I am also guessing that (x union y) should just be a set and not a set of a set

@ozgurakgun
Copy link
Contributor

I am also guessing that (x union y) should just be a set and not a set of a set

I was more worried about this one, yes.

@ozgurakgun
Copy link
Contributor

I think yours is significantly more readable, it's brilliant. I don't see any reason to keep the json one, it'll just be confusing.

Great - do you want to remove the son version then, please?

@ca1umac
Copy link
Contributor Author

ca1umac commented Mar 13, 2026

I am also guessing that (x union y) should just be a set and not a set of a set

I was more worried about this one, yes.

indeed...

@ca1umac
Copy link
Contributor Author

ca1umac commented Mar 13, 2026

I think yours is significantly more readable, it's brilliant. I don't see any reason to keep the json one, it'll just be confusing.

Great - do you want to remove the son version then, please?

Can do!

@ozgurakgun
Copy link
Contributor

I think yours is significantly more readable, it's brilliant. I don't see any reason to keep the json one, it'll just be confusing.

Great - do you want to remove the son version then, please?

Can do!

I don't want to leave this unmerged for a long time, so just removed the json option. Will merge soon.

@ozgurakgun ozgurakgun merged commit 3f5f642 into main Mar 13, 2026
10 checks passed
@ozgurakgun ozgurakgun deleted the testing-new-domain-output-format branch March 13, 2026 14:01
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