Improve download_bee_download_project to support the update command

[yorkshire-pudding]

As discussed in #299 and #297 we need some improvements to the download_bee_download_project() function to better support the update function.

@hosef said:

As for the download command, I think that it would be a good idea to make a more generic download function that has options passed into it instead of accessing global variables or command line arguments directly. Off the top of my head, I think it would need: project name, download destination, version, if it should replace an existing version, if it should make a backup of the project. While we are at it, we should have other functions that handle getting paths and handling version info. We could then make all the commands that need to download projects wrap these other functions so that they behave consistently.

[yorkshire-pudding]

I have added a PR that adds greater flexibility for the internal download_bee_download_project() function, and the download_bee_get_project_info() function that will help to improve the 'update' command.

I realised that if we are to have tests for the 'update' command, and indeed have tests that test the more flexible capability then we will need to be able to download releases other than 'latest' as per #56 and it made sense to include within one PR.

The PR is not ready as the tests need work, but I wanted to get some feedback on the approach.

• The 'select' option provides a list of up to 30 releases (Github API default page) but is this too many (e.g. for dl-core)? Is there a use case for having any more than say the last 10 releases?
• So far there has been no testing of the replace and backup parameters; this will come with the merge into the update branch and revising the update function to make use of this. This is much simpler than the core Backdrop updater process but is it robust enough?

@hosef in particular, I would like your feedback as we discussed the need for this while looking at #347

[indigoxela]

The "select" feature's cool. 👍 Love it!

I belief, a list of 30 is way too much. Even for dl-core. That would be ancient versions, probably nobody ever needs those. 10 seems like a good number to me.

Now some of my Friday testing (caution, Friday is typo day...):

bee dl tinymce:selct
The release you have specified (selct) for 'backdrop-contrib/tinymce' does not exist. Do you want to select a version instead? (Y/n):

Maybe validate for a pattern like "starts with number" - just some plausibility check? Just an idea... not too important.

Oh, this Friday admin didn't read the docs... 😇

# bee dl-core:select
 ✘  There is no 'dl-core:select' command.

Now for real:

bee dl-core --version=select

I'm assuming, there's a reason for this different handling between core and contrib. Anyway, it works. 👍

Two suggestions come to my mind:

1. I'm sitting in front of two columns with numbers, and I'm prompted to "Enter a number" - a bit confusing (which of the numbers). Wouldn't it be OK to just display the version? I know, a number is less to type, but... a bit confusing.
2. How about adding an option to abort? OK, there's ctrl+c but... just for completeness.

Just two quick ideas after a little testing - for your kind consideration @yorkshire-pudding

Bee just rocks, BTW. 😄

[indigoxela]

Another quick idea: if the module already exists, bee dl foobar stops immediately with "... already exists...". Same if the version is provided directly like bee dl foobar:1.x-123.

But the version select does that check after one selects the version. Would it be difficult, to make the check the first step also for the "select" feature?

[yorkshire-pudding]

The "select" feature's cool. 👍 Love it!

😊

I belief, a list of 30 is way too much. Even for dl-core. That would be ancient versions, probably nobody ever needs those. 10 seems like a good number to me.

Great. Confirms what I was leaning towards.

Now some of my Friday testing (caution, Friday is typo day...):

bee dl tinymce:selct
The release you have specified (selct) for 'backdrop-contrib/tinymce' does not exist. Do you want to select a version instead? (Y/n):

Maybe validate for a pattern like "starts with number" - just some plausibility check? Just an idea... not too important.

Yes, could do. While I think I would still give them the same choice to select a version the message could be tailored to whether they were trying to define a release or not.

Oh, this Friday admin didn't read the docs... 😇

# bee dl-core:select
 ✘  There is no 'dl-core:select' command.

Now for real:

bee dl-core --version=select

I'm assuming, there's a reason for this different handling between core and contrib. Anyway, it works. 👍

Yes there is a reason:
dl-core will only download one project (i.e. backdrop) whereas dl can download multiple. I actually started off with the same approach and even separate options for version and branch but then thought "What if someone wants to downloads a few projects and wants to specify version for one or more? How will we associate the options with the project?"

It would require more of a rewrite to allow command names/aliases to have arguments appended to them with the colon. I've tried to keep the format similar so for core --version=branch:1.29.x is similar to paragraphs:branch:1.x-1.2

I'm sitting in front of two columns with numbers, and I'm prompted to "Enter a number" - a bit confusing (which of the numbers). Wouldn't it be OK to just display the version? I know, a number is less to type, but... a bit confusing.

This would involve re-writing bee choice()

How about adding an option to abort? OK, there's ctrl+c but... just for completeness.

Yes, that would be do-able

Another quick idea: if the module already exists, bee dl foobar stops immediately with "... already exists...". Same if the version is provided directly like bee dl foobar:1.x-123.

But the version select does that check after one selects the version. Would it be difficult, to make the check the first step also for the "select" feature?

Might seem like immediately but it's a bit further in the code. However, you make a good point and there is no reason why the check can't come in earlier.

Thank you for all your feedback.

[indigoxela]

Thank you for all your feedback.

Thank YOU for your constant work on Bee. 🙏

Just pick from my feedback, what seems feasible/useful. No need to rewrite even more. 😉