Skip to content

Conversation

s-and-witch
Copy link
Collaborator

Problem: There is no way to reuse the result of diff algorithm because program print it into stdout in human readable format in-place.

Solution: divide steps of finding diff between two derivations and printing it out. Now result of diff is tree with result of derivation comparison, that you can print than in stdout as was before, or use in your program.

@SuperSandro2000
Copy link

How much work would it be to now pipe the diff into diff -u or something similar to have a more powerful diff algorithm?

@s-and-witch
Copy link
Collaborator Author

@SuperSandro2000 do you want to diff sources and environments in other way? In that case, you need to change diffText function in code or restore content of files from diff tree, because all files in the resulting tree are already diffed. You can also try to use --character-oriented CLI flag, as I can see, it will produce more precise diffs of files.

@s-and-witch
Copy link
Collaborator Author

@rvem what do you think about adding Generic and Data instances to all types of tree?

@rvem
Copy link
Member

rvem commented Oct 17, 2022

what do you think about adding Generic and Data instances to all types of tree?

What are the use-cases for them? 🤔

@s-and-witch
Copy link
Collaborator Author

s-and-witch commented Oct 17, 2022

What are the use-cases for them?

We provide Diff tree functionality as library, so it will be great to have possibility to make some orphan instances with generic / use uniplate on resulting tree. But now I understand, that user can make orphan instances of Generic/Data, if he wants.

@SuperSandro2000
Copy link

@SuperSandro2000 do you want to diff sources and environments in other way? In that case, you need to change diffText function in code or restore content of files from diff tree, because all files in the resulting tree are already diffed. You can also try to use --character-oriented CLI flag, as I can see, it will produce more precise diffs of files.

I basically want to fix Gabriella439#42

@s-and-witch
Copy link
Collaborator Author

s-and-witch commented Oct 17, 2022

I basically want to fix Gabriella439#42

You can count offsets and print only parts that differ with calculated position. I think, it can be easily done in renderText function by adding new flag to the render ReaderT environment and changing algorithm of text building.

@SuperSandro2000
Copy link

It would be easy if I would know a thing about Haskell 🙃

@@ -0,0 +1,1014 @@
- /nix/store/5kwdw9nynm8195n9dvn6zzx0rbxb2cvm-nixos-system-nixos-22.11.20221013.cfea568.drv:{out}
Copy link
Member

Choose a reason for hiding this comment

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

Does the upstream master nix-diff version produce the same output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I tested on version from nixpkgs

Copy link
Member

Choose a reason for hiding this comment

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

Also, does this test cover all possible diff elements, i.e. there are different inputs, outputs, environment parts?

Copy link
Collaborator Author

@s-and-witch s-and-witch Oct 19, 2022

Choose a reason for hiding this comment

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

Also, does this test cover all possible diff elements, i.e. there are different inputs, outputs, environment parts?

I'm not sure about that.

Yes, I tested on version from nixpkgs

I start to test it one another time now and get difference. Replaced cmp -u with diff and got this output

38c38
< 
---
>           
44c44
< 
---
>           
46,48c46,48
< 
< 
< 
---
>           
>           
>           
58c58
< 
---
>           
...

So I need to understand, what is the source of difference and replace cmp -u with diff, because it helps us to get more sensible error.

Copy link
Member

@rvem rvem Oct 19, 2022

Choose a reason for hiding this comment

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

I'm not sure about that.

I think it's important to have tests that cover all possible cases, so please check and update tests if needed 🙃

@@ -0,0 +1,9 @@
#!/usr/bin/env bash

export OLD_DRV="$(nix-instantiate ./old-derivation/drv.nix)"
Copy link
Member

Choose a reason for hiding this comment

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

A small nitpick: you probably don't need to export them, old_drv="$(nix-instantiate ./old-derivation/drv.nix)" will be just enough to make it available in the scope of the remaining script, and AFAICS, these env variables aren't used anywhere in the subsequent calls

Problem: There is no way to reuse result of diff alghorithm because
program print it into stdout in human readable format in-place.

Solution: divide steps of finding diff between two derivations and
printing it out. Now result of `diff` is tree with result of derivation
comparison, that you can print than in stdout as was before, or use in
your program.
Problem: we need to check if code changes will also change the behavior

Solution: add test, that will make two derivations and check if output
of program will be different of saved one.
@s-and-witch s-and-witch force-pushed the Player205/#58-Add-intermediate-representation branch from 21d3e97 to f5c009b Compare October 26, 2022 07:07
@s-and-witch s-and-witch merged commit a5fcd96 into main Oct 26, 2022
@s-and-witch s-and-witch deleted the Player205/#58-Add-intermediate-representation branch October 26, 2022 07: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.

3 participants