search

dynamicdan / sn-filesync

2-way sync ServiceNow field values to local files
MIT License
66 stars 37 forks source link

geekLayla's Diffing #44

Closed benthomasknight closed 7 years ago

benthomasknight commented 7 years ago

This is a version of geekLaylas diffing tool. I noticed that it works well, but you had a few issues with it filling up the app.js file. I have stripped out a few sections and made it slightly more modular in the hopes that you will be happy to add it to the main branch.

I have also added a config option for adding in your own diffing engine. It works well with vscode but I have not tried any others.

I won't call it a pretty implementation as it was mostly just moving about geekLaylas code out of the app.js file, but I hope you are happy enough with it.

Any feedback or problems I should address?

Thanks

dynamicdan commented 7 years ago

Also, can you push to the experimental branch instead of master? I don't think I can choose the branch.

benthomasknight commented 7 years ago

Is Sync: The idea of this option is that some diff tools do not have sync options of diffing files. This means that if the user chooses one of these then we cannot clean up the files straight away. I am happy to force the user to use synchronous diffing tools if you like, as it makes it much more simple for us, but unless we do that we have no way of knowing if the command is synchronous unless the user tells us.

For instance, opendiff exits immediately after the comparison request has been sent to FileMerge, meaning it is not synchronous. If we clean up the files then we are removing the files that FileMerge will be looking for.

VSCode, on the other hand, has a -w command which means it will not exit until you close the window, meaning when it closes we can know that the files are no longer needed. Without the -w command it is not synchronous like opendiff above.

Overwriting files: We currently need the option to overwrite the instance version, as we don't have a marker for when a file has been merged. The tool will notice a file has changed and run the diff, but after the merge it will again notice the change and run the diff again. We will need to add a marker to say that a file has already been merged if the overwrite options are going to be removed.

OS: I only have access to windows so it has not been tested on other devices.

Diff tool: The diff tool can be set through a command line argument, meaning any diff tool that can be called from the command line can be used.

Temp files: Many diff tools require files to diff, rather than just sending the contents of files. This means that when we do a diff, we need the local copy and instance copy to exist on the local machine, so we create a new file of the current instances version.

dynamicdan commented 7 years ago

I will give this a spin and see how it ticks. This update will solve one of the last few gaps in terms of comprehensive support. Thanks for your good work and supporting the community!