Add `--only` parameter to filter files on `theme push`/`theme pull` commands #1892

karreiro · 2022-01-05T11:59:19Z

WHY are these changes introduced?

Currently, users can't filter files when they execute theme pull (#1558) or theme push. This PR introduces a new -o/--only parameter to take care of that.

WHAT is this pull request doing?

This PR introduces the ShopifyCLI::Theme::IncludeFilter class, which behaves similarly to the ShopifyCLI::Theme::IgnoreFilter, but includes files that match with the query instead of excluding them.

How to test your changes?

Run the following commands for testing the new --only parameter and notice that all files will be ignored, except the ones in the templates directory:

~$ DEBUG=1 shopify-dev theme pull -t THEME_NAME --only "templates/*"
~$ DEBUG=1 shopify-dev theme push -t THEME_NAME --only "templates/*"

Post-release steps

Update documentation with the new -o/--only=PATTERN parameter: https://shopify.dev/themes/tools/cli/theme-commands#push, https://shopify.dev/themes/tools/cli/theme-commands#pull.

Update checklist

I've added a CHANGELOG entry for this PR (if the change is public-facing)
I've considered possible cross-platform impacts (Mac, Linux, Windows).
I've left the version number as is (we'll handle incrementing this when releasing).
I've included any post-release steps in the section above.

karreiro · 2022-01-10T14:06:03Z

👋 Hello, @MeredithCastile! :)

In many command-line tools, the flag -if stands for --force to perform some operation without confirmation.

To avoid using -f for the filter (so, we may have something like shopify theme push --force in the future), I've adopted the -q/--query in this PR.

Still, I'm not entirely sure it's a good idea 😅 What do you think about this?

Thanks a lot!

MeredithCastile · 2022-01-12T08:11:27Z

Hmm, I'm not sure. Filter seems more intuitive and comprehensible to me than query.

One thing your questions have raised for me, @karreiro, is whether or not we need a shortcut/alias for every flag. If we support only full words for rarer flag, then we could have more than one starting with the same letter.

@pedropinerashopify, curious if you agree with the idea that every flag does NOT need a one-letter shortcut?

karreiro · 2022-01-14T14:02:10Z

(cc @pepicrft)

Generally, when I'm using command-line tools, I usually use the shortcuts, and when the commands get too lengthy, I tend to create aliases for them (but this is a total personal use/opinion).

pepicrft · 2022-01-18T15:08:51Z

I agree that --force/-f is extensively used across CLIs to mean "force" and I'd refrain from using it for querying. Note that there's the option to have lower and upper-cased aliases so we could go with something like:

--force / -f
--filter / -F

I've seen CLIs doing that as a way to disambiguate and I think it's something we can adopt here too.

karreiro · 2022-01-18T16:49:16Z

🤯 --filter, -F seems like an excellent approach! Thanks for that idea, @pepicrft!

MeredithCastile · 2022-01-19T17:40:41Z

Ah, that's such an interesting idea, @pepicrft. So then I suppose there are 2 possibilities (unless we widen the scope of this issue):

POSSIBILITY 1: case-sensitive shortcuts

--force / -f
--filter / -F

Pro: More intuitive language
Con: People are going to inevitably try to filter and accidentally force

POSSIBILITY 2: just go with query

--query / -q

Pro: No risk of the "user error" stated above
Con: Less intuitive command

__

I don't know how disruptive an accidental force command would be, so I'm not well-positioned to weigh between these options.

macournoyer · 2022-01-20T16:14:49Z

Just throwing another option out (sorry 😄)

What about -o / --only [PATTERN]. Seems clearer to me that it will filter to only include those files. Where --filter makes me think of "filter out", this reads better to me:

shopify theme pull --only *.js

karreiro · 2022-01-20T16:26:16Z

No reason to be sorry! This approach is awesome :) It makes the command much more expressive!

Thanks a lot, @macournoyer 🚀

PS.: I believe we may keep case-sensitive shortcuts in mind for future scenarios, but for this PR, I believe the --only offers the best DX :)

…pull` commands

MeredithCastile · 2022-01-21T17:12:57Z

@macournoyer Such an elegant solution! Thank you!

jibinycricket · 2022-01-28T14:42:28Z

Is there a way to pass multiple --only paramaters similar to --ignore?

shopify theme pull -d -n -o 'templates/*.json' -o 'config/settings_data.json'

Right now it only pulls the last only option which is config/settings_data.json

blanklob · 2022-01-31T23:40:41Z

Is there a way to pass multiple --only paramaters similar to --ignore?

shopify theme pull -d -n -o 'templates/*.json' -o 'config/settings_data.json'

Right now it only pulls the last only option which is config/settings_data.json

@jibinycricket You can use the command shopify theme pull -d -n --only "**/*.json"

…me pull`/`push` - updates work from Shopify#1892 - see comment Shopify#1892 (comment)

kilgore5 · 2022-02-02T03:04:05Z

PR addressing #1892 (comment).... #2002

These updates fix an issue where a local `package.json` file was being deleted despite being in the `.shopifyignore` file. - update `download_file!` method to use similar "ignore" handling as the `enqueue` method... using `ignore?` method added in Shopify#1892 - extract `ignore_path?` method from `ignore?` method so it can be called separately from an operation - update `ignore_path?` to check for a File object and extract the path from that

karreiro linked an issue Jan 5, 2022 that may be closed by this pull request

Ability to filter files on pull #1558

Closed

karreiro force-pushed the fix-1558 branch from 9193dad to 0f1a892 Compare January 19, 2022 12:34

karreiro marked this pull request as ready for review January 19, 2022 12:34

karreiro requested review from a team, jesalerno84, gonzaloriestra and macournoyer and removed request for a team January 19, 2022 12:34

karreiro changed the title ~~Add --query parameter to filter files on theme push/theme pull commands~~ Add --filter parameter to filter files on theme push/theme pull commands Jan 19, 2022

macournoyer approved these changes Jan 19, 2022

View reviewed changes

karreiro added 2 commits January 21, 2022 14:59

Add -F/--filter parameter to filter files on theme push/`theme …

06f87a0

…pull` commands

Rename -F/--filter to -o/--only

cdf50fc

karreiro force-pushed the fix-1558 branch from 0f1a892 to cdf50fc Compare January 21, 2022 14:10

karreiro changed the title ~~Add --filter parameter to filter files on theme push/theme pull commands~~ Add --only parameter to filter files on theme push/theme pull commands Jan 21, 2022

karreiro merged commit f017613 into main Jan 24, 2022

karreiro deleted the fix-1558 branch January 24, 2022 08:54

hannachen mentioned this pull request Jan 25, 2022

Packaging for release v2.10.0 #1970

Merged

kilgore5 pushed a commit to kilgore5/shopify-cli that referenced this pull request Feb 1, 2022

Update -o/--only option to allow multiple options when using `the…

d28aa62

…me pull`/`push` - updates work from Shopify#1892 - see comment Shopify#1892 (comment)

kilgore5 mentioned this pull request Feb 2, 2022

[ feature ] Update theme pull/push --only option to accept multiple patterns #2002

Merged

6 tasks

kilgore5 mentioned this pull request Feb 9, 2022

Fix Theme::Syncer download_file! handling of file deletions #2030

Merged

4 tasks

PaulNewton mentioned this pull request Mar 1, 2022

Local customized theme's data not being saved in settings_data.json #1434

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `--only` parameter to filter files on `theme push`/`theme pull` commands #1892

Add `--only` parameter to filter files on `theme push`/`theme pull` commands #1892

karreiro commented Jan 5, 2022 •

edited

Loading

karreiro commented Jan 10, 2022

MeredithCastile commented Jan 12, 2022

karreiro commented Jan 14, 2022 •

edited

Loading

pepicrft commented Jan 18, 2022 •

edited

Loading

karreiro commented Jan 18, 2022 •

edited

Loading

MeredithCastile commented Jan 19, 2022 •

edited

Loading

macournoyer commented Jan 20, 2022

karreiro commented Jan 20, 2022

MeredithCastile commented Jan 21, 2022

jibinycricket commented Jan 28, 2022 •

edited

Loading

blanklob commented Jan 31, 2022

kilgore5 commented Feb 2, 2022

Add --only parameter to filter files on theme push/theme pull commands #1892

Add --only parameter to filter files on theme push/theme pull commands #1892

Conversation

karreiro commented Jan 5, 2022 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Update checklist

karreiro commented Jan 10, 2022

MeredithCastile commented Jan 12, 2022

karreiro commented Jan 14, 2022 • edited Loading

pepicrft commented Jan 18, 2022 • edited Loading

karreiro commented Jan 18, 2022 • edited Loading

MeredithCastile commented Jan 19, 2022 • edited Loading

POSSIBILITY 1: case-sensitive shortcuts

POSSIBILITY 2: just go with query

macournoyer commented Jan 20, 2022

karreiro commented Jan 20, 2022

MeredithCastile commented Jan 21, 2022

jibinycricket commented Jan 28, 2022 • edited Loading

blanklob commented Jan 31, 2022

kilgore5 commented Feb 2, 2022

Add `--only` parameter to filter files on `theme push`/`theme pull` commands #1892

Add `--only` parameter to filter files on `theme push`/`theme pull` commands #1892

karreiro commented Jan 5, 2022 •

edited

Loading

karreiro commented Jan 14, 2022 •

edited

Loading

pepicrft commented Jan 18, 2022 •

edited

Loading

karreiro commented Jan 18, 2022 •

edited

Loading

MeredithCastile commented Jan 19, 2022 •

edited

Loading

jibinycricket commented Jan 28, 2022 •

edited

Loading