backdrop-contrib / brush

Brush is a command line shell and Unix scripting interface for Backdrop CMS.
GNU General Public License v3.0
2 stars 0 forks source link

Rename vset commands respectively to config-list, config-set, config-get commands #2

Closed AlexShapka closed 4 years ago

AlexShapka commented 5 years ago

Subj.

alanmels commented 5 years ago

In fact, because Brush is based on drush 4.x there is already vset command, so we just need to rename it to bring up to Backdrop terms.

ghost commented 4 years ago

Is it only a rename (or API only change)? [I have time for that] Or is it a complete code re-write? [Don't have time for this though :( ]

I had to fix one of my alias that used Drupal variables (Backdrop configs) and it entailed a code change as B-configs are now flat files and not in the DB like D-variables.

Best, Michael

alanmels commented 4 years ago

The variable_get, variable_set, and variable_del commands, which drush relied upon, are deprecated by CMI. Therefore, the commands in variable.brush.inc file have to be re-written using:

https://api.backdropcms.org/api/backdrop/core%21includes%21config.inc/function/config_get/1 https://api.backdropcms.org/api/backdrop/core%21includes%21config.inc/function/config_set/1 https://api.backdropcms.org/api/backdrop/core%21includes%21config.inc/function/config_clear/1 https://api.backdropcms.org/api/backdrop/core%21includes%21bootstrap.inc/function/state_get/1 https://api.backdropcms.org/api/backdrop/core%21includes%21bootstrap.inc/function/state_set/1 https://api.backdropcms.org/api/backdrop/core%21includes%21bootstrap.inc/function/state_del/1

instead.

ghost commented 4 years ago

Thanks Alan,

Working on it. It seems like more than just variable.brush.inc

Ref:

michael@local [~/data/trash/brush/brush-1.x-1.3]# grep -ir 'variable_get'
includes/command.inc:        // Too early for variable_get('install_profile', 'default'); Just use default.
includes/command.inc:        if ($profile = variable_get('install_profile', NULL)) {
brush.php:    $name = $user->name ? $user->name : variable_get('anonymous', t('Anonymous'));
commands/pm/update_info/backdrop.inc:  return variable_get('update_last_check', 0);
commands/core/search.brush.inc:  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/search.brush.inc:    foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/variable.brush.inc:function brush_variable_get() {
commands/core/upgrade.brush.inc:  $admin_theme = variable_get('admin_theme', NULL);
commands/core/core.brush.inc:    'profiles/default/modules' => "/^$target\.module$/", // Too early for variable_get('install_profile', 'default'); Just use default.
tests/userTest.php:    $eval = "require_once BACKDROP_ROOT . '/' . variable_get('password_inc', 'includes/password.inc');";
alanmels commented 4 years ago

Hi Michael,

If you gonna work on this please make sure to git clone the latest dev branch as I am constantly pushing various fixes.

alanmels commented 4 years ago
michael@local [~/data/trash/brush/brush-1.x-1.3]# grep -ir 'variable_get'
includes/command.inc:        // Too early for variable_get('install_profile', 'default'); Just use default.
includes/command.inc:        if ($profile = variable_get('install_profile', NULL)) {
brush.php:    $name = $user->name ? $user->name : variable_get('anonymous', t('Anonymous'));
commands/pm/update_info/backdrop.inc:  return variable_get('update_last_check', 0);
commands/core/search.brush.inc:  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/search.brush.inc:    foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
commands/core/variable.brush.inc:function brush_variable_get() {
commands/core/upgrade.brush.inc:  $admin_theme = variable_get('admin_theme', NULL);
commands/core/core.brush.inc:    'profiles/default/modules' => "/^$target\.module$/", // Too early for variable_get('install_profile', 'default'); Just use default.
tests/userTest.php:    $eval = "require_once BACKDROP_ROOT . '/' . variable_get('password_inc', 'includes/password.inc');";

You won't find that many occurrences on development branch. Please see https://github.com/backdrop-contrib/brush/commit/e8cb73edd36b068726b7aa1378dce2768df2e601

ghost commented 4 years ago

Okay,

Cloned the dev branch to work with. *

I am amused by the addition of state_get, which pretty much invalidates the rational for converting variable_get to config_get?

This issue also seems dependent on: https://github.com/backdrop/backdrop-issues/issues/1954

While this is convertible:

alias lastcron='echo `drush vget cron_last --exact | awk '\''{print $2}'\''` | { read a; date -d @$a; }'

I’d very much like to not remove the aliases to ‘vget’ (vset, ...) so the above is convertible by just changing drush to brush.

Granted I’m not sure how to work that with cron_last now being part of state_ not config_ :(

In any event forcing a user to know what the JSON file name is for some variable they need to look up is fairly counterproductive. It’d suck though to have to use a disk scan, although for a temporary measure it’d be okay I guess.

I’ll look into it more tomorrow.

Best, Michael

PS: A request. Please don’t remove the old Drupal comments. An example is:

/brush-1.x-1.3/commands/core/upgrade.brush.inc Line: 500 // http://drupal.org/node/724102 recommends using "seven" as your admin theme. Don't switch it to garland if it is already seven.

I agree it’s not a Backdrop ‘thing’ anymore but it does show why it’s coded the way it’s coded. Painless to leave it in and will curtail having to explain it in the future.

alanmels commented 4 years ago

Thanks for your comments. I'll definitely take them all into my attention. For instance, you've changed my mind about leaving the original comments. Because, before I believed that eventually the code could be totally polished, cleaned up and left with only Backdrop related code, including the terminology and the lingua of the comments.

I am amused by the addition of state_get, which pretty much invalidates the rational for converting variable_get to config_get?

If I am not mistaken, the Backdrop consensus decided to differentiate between variable_get:

Returns a persistent variable.

Case-sensitivity of the variable_* functions depends on the database collation used. To avoid problems, always use lower case for persistent variable names.

This function is deprecated in Backdrop and will be removed in a future release. Variables are not managed through the configuration system, so they cannot be moved between environments safely. If your module needs to save configuration settings, use config_get() instead. If you need to save an environment-specific setting (such as the last time cron ran), use state_get() instead.

and state_get():

Retrieves a "state" value from the database.

States are used for saving long-term values in the database that are environment-specific. Some examples of values appropriate for utilizing states include the last time cron was run or the current query string appended to CSS/JS files. These values should not be copied between environments, so they are stored in the state system instead of as configuration files.

Case-sensitivity of the state_* functions depends on the database collation used. To avoid problems, always use lower case for persistent state names.

I’d very much like to not remove the aliases to ‘vget’ (vset, ...) so the above is convertible by just changing drush to brush.

I agree. Let's change only drush to brush leaving the rest, including the arguments, intact.

Granted I’m not sure how to work that with cron_last now being part of state_ not config_ :(

Unfortunately, we at AltaGrade are busy with lot's of our own - mostly system administration, bus sometimes Backdrop - tasks. Only when we hit the point we need certain feature from brush we can afford spending time working on it. The most urgent feature for us was getting database replications using command line, so we pushed brush forward and then rarely touched it. Unfortunately, I don't think our team will need anything from brush cron-related. So if you could please research on this and file a PR, and I'd gladly review it.

You wanna become co-maintainer? You are really welcome to join in and improving the code. I really though b is the right way to go. However, if someone is using this tool and wants to further support it, then I'll be only thankful and try to be helpful.

In any event forcing a user to know what the JSON file name is for some variable they need to look up is fairly counterproductive. It’d suck though to have to use a disk scan, although for a temporary measure it’d be okay I guess.

You could ask on https://backdrop.zulipchat.com and I am pretty sure everyone will be helpful. I think we could add a new feature to brush, that enables quickly finding the right JSON file. Would be totally different from drush and so useful to Backdrop.

* Thanks for the command earlier, that saved me a good bit of time.

Which one, I now wonder?

ghost commented 4 years ago

Hi Alan,

I already did a wall of text today, so I’ll try to keep this short :(

we at AltaGrade are busy

Fully understand that, business owner myself! Time spent on projects like this are time not spent on something billable. :(

You wanna become co-maintainer?

I don’t mind. Do understand I don’t know git (granted I can read docs), so I’d much rather do PRs like the last one so someone who actually knows git can at least review what I’ve changed.

Backdrop Console

That’s a third Drush clone? The only other one I knew about was ??? which gave instructions using Lando to install it.

Ah, ‘b’ as the executable name? Not really a good sign. I’m still going to try it, I need to update a dev Backdrop/CiviCRM site. But based on the replies I’m getting in the other thread, it’s not going to do vget the way I want, so I’ll be sticking with brush for the long term.

we [got] brush [working for what we needed] and then rarely touched it. I think we could add a new feature to brush, that enables quickly finding the right JSON file.

Agree with both of these. I use drush 5.x on all my production servers running D7 because I never needed any of the later ‘stuff’ and why waste the time (and wondering about potential bugs) up-reving drush?

My goals are:

As far as I’ve tested, brush does everything else I want.

Which one, I now wonder?

The git clone for pulling the HEAD. New software, and this applies to about any software, the simplest things are just ‘known.’ Probably like the auto re-versioning, I found references that it could be done, but no real concrete methods...

Okay, got to go, busy for the rest of the day, you be well.

Best, Michael

alanmels commented 4 years ago

You wanna become co-maintainer?

I don’t mind. Do understand I don’t know git (granted I can read docs), so I’d much rather do PRs like the last one so someone who actually knows git can at least review what I’ve changed.

I totally forgot that unlike drupal.org (where we could add co-maintainers any time by ourselves), all the Backdrop projects here belong to https://github.com/backdrop-contrib, and you have to pass through a procedure in order to become a co-maintainer. Please read https://backdropcms.org/contribute/join for the detailed information. If you feel it's too early for you then you can keep contributing just making PRs.

That’s a third Drush clone? The only other one I knew about was ??? which gave instructions using Lando to install it.

Please read README.md on https://github.com/backdrop-contrib/brush to see the differences between consoles.

But based on the replies I’m getting in the other thread, it’s not going to do vget the way I want, so I’ll be sticking with brush for the long term.

That kind of differences were one of the reasons to come up with brush without bothering developers of other two consoles.

My goals are:

* get `brush update` to work

* get vget to work (e.g. finding the right JSON files)

* make `brush vget` transparent to config_ and state_

Well, brush up module already works fine on dev, the only thing it's failing to correctly do is updating core. But I will spend some time today on this and get it sorted out, so hopefully today we'll have fully functional update feature. As for re-working the code to deal with JSON files will take some more time.

The git clone for pulling the HEAD.

Now I see. HEAD will have all the changes that we've pushed recently, so if you are a developer then it's better to have fresher code. But then you don't have to run git clone every time you need the latest state - though that's one of the legitimate ways, I usually do git pull to sync my local branch. Anyway, you'll quickly get used to git ways of doing things.

New software, and this applies to about any software, the simplest things are just ‘known.’ Probably like the auto re-versioning, I found references that it could be done, but no real concrete methods...

Don't worry about it as it's fixed on dev branch. In other words, brush is showing the correct version on CLI.

alanmels commented 4 years ago

This has partially been fixed. Please see https://github.com/backdrop-contrib/brush/issues/12

alanmels commented 4 years ago

brush cset (alias brush vset) command started to work on dev after https://github.com/backdrop-contrib/brush/commit/3ab4447e67fe53d3aa4f6b1dcbbdbc210df85e79

alanmels commented 4 years ago

This has finally been fixed. Here are outputs for four newly introduced commands:

brush help clist
Display a list all configuration files.

Examples:
+-------------------------------+---------------------------------------------------------------------------+
| brush config-list             | List all configuration files.                                             |
| brush config-list user        | List all configuration files containing the string "user" in their names. |
+-------------------------------+---------------------------------------------------------------------------+

Arguments:
+-------------------------------+------------------------------------------------+
| name                          | A string to filter the configuration files by. |
+-------------------------------+------------------------------------------------+

Options:
+-------------------------------+------------------------------------------------------------------------+
| --pipe                        | Use var_export() to emit executable PHP. Useful for pasting into code. |
+-------------------------------+------------------------------------------------------------------------+
docker@cli:/var/www/docroot$ brush help cget
Get a list of some or all site variables and values.

Examples:
+----------------------------------------+------------------------------------------------------------------+
| brush config-get                       | List all variables and values.                                   |
| brush config-get system.core           | List all variables in "system.core.json" file.                   |
| brush config-get system.core clean_url | Display value of "clean_url" key in "system.core.json" file.     |
| brush cget entity_type                 | List values of the "entity_type" key in all configuration files. |
| brush cget *entity*                    | List variables whose keys contain string "entity".               |
+----------------------------------------+------------------------------------------------------------------+

Arguments:
+-------------------------------+-----------------------------------------------------------+
| config-name                   | The configuration object name, for example "system.core". |
| key                           | The config key, for example "clean_url".                  |
+-------------------------------+-----------------------------------------------------------+

Options:
+-------------------------------+------------------------------------------------------------------------+
| --pipe                        | Use var_export() to emit executable PHP. Useful for pasting into code. |
+-------------------------------+------------------------------------------------------------------------+

Aliases: cget, vget
docker@cli:/var/www/docroot$ brush help cset
Set a configuration key.

Examples:
+--------------------------------------------------+-----------------------------------------------------------------------------------+
| brush config-set --yes preprocess_css 1          | Set the preprocess_css variable to true.                                          |
| brush cset system.core                           | Set severity of logs to Emergency & Alert. See /admin/config/development/logging. |
| watchdog_enabled_severity_levels '{"0":0,"1":1}' |                                                                                   |
+--------------------------------------------------+-----------------------------------------------------------------------------------+

Arguments:
+-------------------------------+-----------------------------------------------------------------------------+
| config-name                   | The configuration object name, for example "system.core".                   |
| key                           | The config key, for example "site_slogan".                                  |
| value                         | The value to assign to the config key. Arrays can be input as JSON strings. |
+-------------------------------+-----------------------------------------------------------------------------+

Options:
+-------------------------------+------------------------------------------------------+
| --yes                         | Skip confirmation if only one variable name matches. |
| --always-set                  | Always skip confirmation.                            |
+-------------------------------+------------------------------------------------------+

Aliases: cset, vset
brush help cdel
Delete a single value from a config file.

Examples:
+-----------------------------------------------+-------------------------------------+
| brush config_clear system.core site_frontpage | Delete the site_frontpage variable. |
+-----------------------------------------------+-------------------------------------+

Arguments:
+-------------------------------+-------------------------------------------------------------+
| config-name                   | The configuration object name, for example "system.core".   |
| key                           | A configuration key to clear, for example "site_frontpage". |
+-------------------------------+-------------------------------------------------------------+

Options:
+-------------------------------+------------------------------------------------------+
| --yes                         | Skip confirmation if only one variable name matches. |
+-------------------------------+------------------------------------------------------+

Aliases: cdel, vdel