Shopify / themekit

Shopify theme development command line tool.
https://shopify.dev/tools/theme-kit
MIT License
1.18k stars 368 forks source link

Ability to diff between local and remote theme files #697

Open lvl99 opened 4 years ago

lvl99 commented 4 years ago

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:

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 commented 4 years ago

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 commented 4 years ago

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:

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:

tanema commented 4 years ago

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

PaulNewton commented 4 years ago

@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?

tanema commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

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
    1. 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: #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 commented 3 years ago

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 commented 3 years ago

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

dan-gamble commented 3 years ago

Thanks for the update @andyw8!

mrpacman101 commented 3 years ago

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 commented 3 years ago

@mrpacman101, the live theme is like a remote pull from an untracked multi-collaborative gitless repo you can never rely on to document itself. Typically when using version control you'd first run theme download on the target theme(production) to merge any upstream changes into your repo before running theme deploy. If you think the sync will be a real problem, give it it's own branch first. Then just figure out how you want to commit the changes, or resolve conflicts. Either as one blob , as categorical commits(settings, locales, media assets, per app changes, analytics etc), or more excessively go granular where you're documenting each change.

For the syncs commit line, or branch, I've been using a format of YYYY-MM-DD sync - category(optional) - remote [LIVE] to local [DEV] - reason(optional) . where [LIVE] and [DEV] try to be tags on the theme names but not always. Along with other common situational themes like [SWING] [Backup], or tagged with another developer|agencies name. Then only add notes to the commit body if there's non normal weirdness and I had to make the commit line long. As syncing is the reason , reason is to differentiate similar commits for the history but I never use it to keep commit line short.

On Wed, Dec 23, 2020 at 4:36 PM Mark notifications@github.com wrote:

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.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Shopify/themekit/issues/697#issuecomment-750483124, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALIKKMLH5O5TOBA3KSCX3Q3SWJPF7ANCNFSM4LDDBGHQ .

mrpacman101 commented 3 years ago

@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?