approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
318 stars 51 forks source link

Data Driven Model for Merge Tools #105

Open jwillikers opened 4 years ago

jwillikers commented 4 years ago

This is just an idea to make adding new merge tools simpler. A lot of the merge tools function quite similarly and their implementations in ApprovalTests each require the creation of at least on class per merge tool. However, given multiple versions, names, or operating systems, the number of classes required increases. This amounts to quite a bit of boilerplate when adding new merge tools. By defining a struct for storing all of the necessary data, construction of one-off classes can be avoided, and this struct can provide all of the necessary details required to find and call a diff tool from within ApprovalTests.

An example struct as follows hopefully encapsulates most of the variance between merge tools.

#include <string>
#include <vector>

enum Context {
  Background = 0;
  Foreground
};

struct DiffTool {
  std::string name;
  std::vector<std::string> search_paths;
  std::vector<std::string> names;
  std::string arguments;
  Context = Context::Background;
  unsigned int priority = 0;
};

By defining a standard search procedure for the search_paths and names, each tool can be found using the same process. The search_paths and names members would likely need to be defined in a way to only include certain values dependent on the OS.

I'm not sure how viable this is, but I wanted to get the idea out there. It would be nice to only have to define most merge tools in one place.

Note: Multiple versions of the same tool would still require special handling.

isidore commented 3 years ago

Hello,

Thanks for the suggestion.

I like the idea of being able to put multiple paths in a DiffInfo, but don't think we will do that until the next time we need to add a reporter that require this feature.

So good idea, but currently on hold.