Vimes.jl icon indicating copy to clipboard operation
Vimes.jl copied to clipboard

Feature request: add a list of diffs to ignore

Open DilumAluthge opened this issue 6 years ago • 3 comments

This is still an idea in my head. Not 100% sure what it would look like.

My basic idea is this: Suppose I run Vimes, and then I look in the .vimes folder and look at the .diff file. I realize that, for whatever reason, I'm not going to address this specific patch right now. So I add an issue to my GitHub issue tracker, to come back to later. I'd like to run Vimes again, but since I have already found this patch, there is no need to Vimes to try this patch again. In fact, if Vimes generates the same patch, it shouldn't even run Pkg.test on it - instead, it should generate a new patch.

So what I am envisioning is that we add a keyword argument to Vimes.go(...). The keyword argument would be named something like ignore_diffs, and an example usage would be Vimes.go("."; ignore_diffs = "/Users/dilum/SomeFolder/"). The /Users/dilum/SomeFolder/ would be full of .diff files. If any of the .diff files in /Users/dilum/SomeFolder/ exactly matched a patch generated by Vimes, that patch will be ignored - Pkg.test won't be run, and the values of runs and pass will not be modified.

Let me know what you think. At some point in the future, I'll try to make a proof-of-concept pull request.

DilumAluthge avatar Jun 09 '19 10:06 DilumAluthge

This seems like a good idea, but I'm not sure how to present it to users either. Diffs are not ideal, since for example a mutation that replaces an Int will almost always produce a different diff, even though it's likely you're uninterested in the whole class of patches.

You can currently specify the list of patches manually, and we could make this per-file, but that seems cumbersome.

Some tools let you write comments that exempt a given block of code. That might work well but is a bit tricky with our current source modification tools since comments don't show up. Maybe we could adjust CSTParser to include comment nodes.

As we add more mutations (and make the existing ones more robust), it's possible that this will just become less of an issue, since repeat patches are pretty unlikely to come up.

MikeInnes avatar Jun 10 '19 12:06 MikeInnes

This seems like a good idea, but I'm not sure how to present it to users either. Diffs are not ideal, since for example a mutation that replaces an Int will almost always produce a different diff, even though it's likely you're uninterested in the whole class of patches.

Perhaps we could get around this by allowing the user to pass in a list of regexes? Something like Vimes.go("."; ignore_regexes = [list, of, regexe]). And then if the patch matches against any regex in the list, we generate a new patch instead.

You can currently specify the list of patches manually, and we could make this per-file, but that seems cumbersome.

Yeah that doesn't seem like the best idea.

Some tools let you write comments that exempt a given block of code. That might work well but is a bit tricky with our current source modification tools since comments don't show up. Maybe we could adjust CSTParser to include comment nodes.

This could be a good idea. I don't know how difficult it would be to make that patch to CSTParser though. Plus we are still waiting for your first patch to CSTParser to get merged...

As we add more mutations (and make the existing ones more robust), it's possible that this will just become less of an issue, since repeat patches are pretty unlikely to come up.

Definitely agree that as we add more mutations this will be less common. Still would be nice to have the feature though.

DilumAluthge avatar Jun 14 '19 16:06 DilumAluthge

It's possible that we'll just end up having to fork CSTParser for stuff like this. Not ideal but what we want is really quite different; it'd be nice to have a higher-fidelity Expr format, as well as representations of comments and docstrings for things like this.

MikeInnes avatar Jun 27 '19 11:06 MikeInnes