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

Treeification of Variation Diffs #67

Merged
merged 38 commits into from
Mar 2, 2023
Merged

Treeification of Variation Diffs #67

merged 38 commits into from
Mar 2, 2023

Conversation

pmbittner
Copy link
Member

This PR introduces BadVDiffs (Bad Variation Diffs) that are an alternative representation of variation diffs as trees instead of directed acyclic graphs. A BadVDiff is a variation diff in which any shared subtrees (i.e., sub-variation-diffs below a diamond structure) are duplicated to make the diff become a tree. This enables us to partly reuse existing functionality and research on variational tree languages, such as partial configurations.

@pmbittner pmbittner added the enhancement New feature or request label Feb 21, 2023
@pmbittner pmbittner self-assigned this Feb 21, 2023
@pmbittner pmbittner changed the base branch from main to develop February 21, 2023 14:57
Copy link
Collaborator

@ibbem ibbem left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me. The only bug I found is in DiffNode.toTextDiff but I request some API and implementation changes. I also added many nitpicks.

Thanks for the good commit hygiene (apart from 40cbe25 which could be split up next time to separate unrelated changes). Although some commit messages could provide a bit more context.

@ibbem ibbem self-requested a review March 1, 2023 11:17
Copy link
Collaborator

@ibbem ibbem left a comment

Choose a reason for hiding this comment

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

Seems good to go. Once the open conversations are resolve this can be merge.

Use `Time.forAll` in `BadVDiff.fromGood`
@pmbittner pmbittner merged commit 10cb260 into develop Mar 2, 2023
@pmbittner pmbittner deleted the vpatch branch March 10, 2023 11:12
@pmbittner pmbittner restored the vpatch branch March 10, 2023 11:12
@pmbittner pmbittner deleted the vpatch branch March 10, 2023 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants