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

Ability to diff between local and remote theme files #697

Open
lvl99 opened this issue Mar 6, 2020 · 13 comments
Open

Ability to diff between local and remote theme files #697

lvl99 opened this issue Mar 6, 2020 · 13 comments

Comments

@lvl99
Copy link

lvl99 commented Mar 6, 2020

Is your feature request related to a problem? Please describe.
We manage our theme codebase in a repo, and often when installing new apps those new apps add new files to the remote theme. It would be great to see what is new and different on the remote compared to the local to understand what to download from the remote to include in the repo.

Also when doing a full theme deployment via theme deploy --env=<name of env> that command will remove any files from the remote that aren't included in the local. Before doing a theme deploy it would make sense for a developer to sanity check for any missing files on their local by first using a theme diff.

Additionally, when sharing development between other developers, it'd be great to see the difference between a local branch versus the remote theme, in case other developers have made changes and haven't communicated them within the team.

Describe the solution you'd like
theme diff [--env=<name of environment>] [--compare-to-env=<name of environment>] [--show-only-missing]

This command would produce a list of files where the local theme differs from the remote, e.g.:

> theme diff --env=development

Fetching differences between local and remote: development theme files...

Local                                  | Remote: development
---------------------------------------+--------------------
assets/theme.js                  4.0kb | +---------    4.1kb
layouts/theme.liquid            24.6kb | +++-------   29.2kb
snippets/example.liquid          1.2kb | ++++------    1.4kb
snippets/new-local-file.liquid   0.9kb | ++++++++++    -----
snippets/only-on-remote.liquid   ----- | ++++++++++    1.5kb

The theme diff output would only list files which have differences:

  • File path/name
  • Size of local vs remote file
  • Representation of changes, e.g. if 4 out of 100 lines changed = 4% changed
  • Additional usage of colours to show if file is changed (orange), new on local (green), new on remote (red)

Additionally to diff between two remotes could be interesting too:

> theme diff --env=development --compare-to-env=production

Fetching differences between remote: development and remote: production theme files...

Remote: development                    | Remote: production
---------------------------------------+--------------------
assets/theme.js                  4.0kb | +---------    4.1kb
layouts/theme.liquid            24.6kb | +++-------   29.2kb
snippets/example.liquid          1.2kb | ++++------    1.4kb
snippets/new-local-file.liquid   0.9kb | ++++++++++    -----
snippets/only-on-remote.liquid   ----- | ++++++++++    1.5kb

The option to show only missing files could be interesting:

> theme diff --env=development --show-only-missing

Local                                  | Remote: development
---------------------------------------+--------------------
snippets/new-local-file.liquid   0.9kb | ++++++++++    -----
snippets/only-on-remote.liquid   ----- | ++++++++++    1.5kb

Describe alternatives you've considered
Currently have been downloading the theme code and then locally comparing branches. It's quite slow to download, and extra manual actions to have to perform the diff locally.

Additional context
Code version software routinely uses diffs for managing changes between commits and branches.

Having the ability to diff between the local and remote version of a theme means the theme developer could quickly see what differences there are, and if they need to download any changes to their local before they do a theme deploy command and potentially break the site.

@louiswalch
Copy link

I support this too. Or similarly a way to tell get, download, deploy commands to only touch files that have been changed? If I'm working locally and forget to activate watch then I have to deploy the entire theme.

@PaulNewton
Copy link

PaulNewton commented May 21, 2020

Related - #330 Detecting remote changes to theme

I don't think that's likely to be a native feature(see below), shorterm I think the solution is some docs on workflow(git commands) &deployment strategies with simple scripts and cli aliases, and tools for some editors to automate more of the micro decisions(such as piping recently changed files as a list to >themekit download .....

So any native command like -diff could just be an alias, or workflow sugar.
Here's a spitball similar to a workflow I use:

  • check for script setting in config.yml OR use default community built scripts(Mac,linux,PC)
    >log script location running
  • check if git is installed >git --version
  • check if there is a repo >git rev-parse --is-inside-work-tree
  • check if working tree is clean >git -diff
    (returns no text when empty)

Pseudo >themekit run diff:

if git && repo && clean
 themekit download -env...
 git -diff
else 
 error|warn git is missing , or working dir is dirty(commit your work or use git's stash command)

A script setting could be similar to npm's run command so instead of >themekit diff it could be >themekit run diff with diff being either full filename or command

Though a big failure point is if git isn't being used then basically the only sane way this would still work is for any download to happen in a different directory so the working directory doesn't get clobbered ,which makes me think that might be a seperate issue to check for that anyway
(note to self: overwrite).
If git diff can't be used then it should fail or default to using Linux's diff commnd and Windows FC command.

AFAIK there are several things preventing such a diff feature from being a hard part of themekit:

  • themes are not handled as repos on shopify (there is a soft file history via the admin)
    if they were git pull would be the daily driver
  • A big assumption is that someone using themekit will also be using git and assumes that is the diffing tool
  • the entire theme still has to be downloaded (like waiting on a build)
  • file overwrites from download,etc are automatic
    so cli workflows w/o git are destructive in the prime folder
    manully duping a theme project to do a diff is clunky
  • theme files via the shopify assets api only has updated_at on the asset api as a publicly documented property.
    -- no checksums
    -- Using timestamps as the logic for fetching diffs is brittle
  • diffing tools aren't consistent across OS's , nor simple to implement

@tanema
Copy link
Contributor

tanema commented May 22, 2020

We are currently working on something to ease this, I cannot say much about it but it is a work in progress.

@PaulNewton
Copy link

PaulNewton commented May 22, 2020

@tanema awesome! I don't have bandwith to try and formalize what I've outlined above into something consumable.
100% get it if you can't comment on this bit but if I could set aside time in the next couple of months would it be kinda pointless at this point?

@Shopify Shopify deleted a comment from willryanuk Jun 15, 2020
@tanema
Copy link
Contributor

tanema commented Aug 31, 2020

We just released v1.1.0. We have just released a checksum value in the asset API (in the unstable api version) and so we have enabled themekit to now only perform actions that are necessary. This means that you can now run theme download and it will only download the files that have changed. In this case, you could diff things yourself really quickly.

However we have also put this functionality into watch and deploy. This means when you are deploying, it will only upload files that have changed, and maybe this will cut down some of the reasons why you wanted a diff command.

Hopefully these extra features have mitigated some of your needs but either way I look forward to your feedback on the new features!

@lvl99
Copy link
Author

lvl99 commented Sep 1, 2020

Sounds great @tanema ! Definitely have mitigated some, but having the checksum in the assets API means there's at least a pathway to providing a diff feature, I think.

@PaulNewton
Copy link

I think the changes mostly from #745 stemming from #714 and other checksum pulls

Great! at least getting only necessary files in a remote theme should speed up sync and diff workflows instead of downloading everything. Though for the download command with empty dir's that speed up is only after the first run gets all the files.

Is there an override|force|hard(opposite --soft)? to skip checksums?
themekit upload * ? as a config property?
When checksums are calculated on remote systems larger files almost always create a user annoyance with this.
And this may cause headaches with line endings (shopify unix, users not always unix) and minification of json files.
sorry cannot invalidate these gripes I'm in the middle of projects and can't change horses

@tanema How does shopify envision themekit's role in workflows such as Diff and syncs?
Remote checksums still don't facilitate safety for local workflows such as diff or sync, currently safety needs extra hoops via duplicate directories(or cache) ,backups,or version control.

With Diff, it's easy to assume a user's who's diffing is advanced enough to know they need to be:

  • using multiple directories while passing in the flags(--env,--dir) with correct values
  • and/or using another tool like GIT and have preserved local changes with commit,stash,etc

My Blah blah
Possibly to ease some of extra theme management hoops locally there needs to be safety's for workflows. A way to get the list of checksums&filenames or to confirm|accept changes knowing what will be overwritten, before nuking local files then making decisions.
--simulate, --test , --confirm ,etc aka risk mitigation parameters.
git has --no-commit &--no-ff, powershell has --WhatIf &--Confirm

Regardless of a diff specific feature, I think the meta problem here is themekits workflows don't enable making informed decisions about changes in advance as the common commands are inherently all or nothing and assumes users are knowledgeable enough to prevent wiping out their work. For beginners like designers, or merchants, tools like this being clinical often mean a painful first-time lesson about file handling & backups.

I guess mostly it's a sentiment where it seems fragile and error prone to be required to first write entire files and to then be able to decide if that was the wrong irreversible choice. I can't even count the number of times i've forgotten to ignore settings_data.json. But really it takes magic for themekit to know user intent and either create a temp dir, or integrate with git to stash any possible local overwrites, and know whether to copy or download the entire theme for the purposes of reconciling distributed file changes.

DIFF Workflow examples (using meld)

Multiple Directories

AFAIK currently the simplest way to work safely with only themekit and a difftool requires using a different directory either through separate cli's, the themekit flag --dir ,or themekits config envs,etc.

Windows command line:

  1. Create a temporary directory(here in the parent directory of the theme)
    C:\theme> mkdir ..\DIFFTEMP
  2. Download possible changes to that temporary directory
    C:\theme> themekit download --env=production --dir=\DIFFTEMP [1]
  3. Diff the changes using ( meld )
    C:\theme> meld . .\DIFFTEMP
  4. When done optionally delete the temp directory so subsequent Diffs from different sources aren't muddied.
    C:\theme>rmdir /S ..DIFFTEMP

[1] themekit will not create a new folder with it's --dir flag it will raise an error invalid environment [development]: (invalid project directory FindFirstFile C:\Sites\TEMPDIFF: The system cannot find the file specified.)

Notes: \DIFFTEMP could be placed in the themes folder but this can lead to new user confusion because of the nesting.
I wonder if an empty directory should get a reminder of existing themes from the config along with OS commands to duplicate local folders &contents?

GIT with a single directory

Users of GIT have more setup steps but get more choices, so as long as local changes are preserved either by commit or stash commands. Then users can safely get whatever themekit downloads for comparing the dirty working directory to the current branch and commit changes. Or "undo" using several options provided by GIT: git checkout,git reset, git revert, etc.

  1. Commit current work
    >git commit -a -m "commit message"
  2. Get remote changes on shopify servers
    >themekit download --env=production --allow-live
    note: Automatically allowing live on open and download #760 obsoletes --allow-live for download command
  3. Internal Diff on the command line itself
    >git diff
    3b. Or external Diff tool if configured[2] , using flag --dir-diff to use folder view instead of a single file at a time.
    >git difftool --dir-diff
  4. Commit desired changes if any
  5. Reset(undo) any remaining changes you don't want to commit on current branch
    >git checkout

[2]How do I set up and use Meld as my git difftool?

@dan-gamble
Copy link

We just released v1.1.0. We have just released a checksum value in the asset API (in the unstable api version) and so we have enabled themekit to now only perform actions that are necessary. This means that you can now run theme download and it will only download the files that have changed. In this case, you could diff things yourself really quickly.

However we have also put this functionality into watch and deploy. This means when you are deploying, it will only upload files that have changed, and maybe this will cut down some of the reasons why you wanted a diff command.

Hopefully these extra features have mitigated some of your needs but either way I look forward to your feedback on the new features!

@tanema do you have any rough ETA on when this would be implemented with node-themekit as well?

@andyw8
Copy link
Contributor

andyw8 commented Sep 1, 2020

@dan-gamble We should have the node-themekit update out in the next day or two.

@dan-gamble
Copy link

Thanks for the update @andyw8!

@mrpacman101
Copy link

hello, will this feature also be available for "theme deploy" ?
We are facing a lot of issues - manually diff remote and local theme version, when Client installs apps or creates custom templates. When i run theme deploy - remote files will be deleted, would it not be better to not delete remote created files?
This would solve so many issues when clients install apps, snippets etc via app autoinstaller.

We want to use Github actions for automatic deployment, but there is no way if we "force" the client to "not" install any apps related to changes in the theme.

@PaulNewton
Copy link

PaulNewton commented Dec 23, 2020 via email

@mrpacman101
Copy link

@PaulNewton
Thanks Paul, i am looking into automating this process as currently i am doing this merging manually. I use as you described throw away branches to grab changes first and merge them - then run a manual deploy.

Maybe i will come up with a small github actions script which would take care of this process. I wonder how other agencies solves this - do they all just use their own custom scripts for continues deployment? Any other tools i might need to look out for like - anyone experienced using this with Beanstalk?

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

No branches or pull requests

7 participants