Added 'status' command for cache cli script / Also improved readability ...

[facundocapua]

...in output messages

[maksek]

HI @fcapua-summa, can you please provide more details in description. Use cases, output format and specifics of "improved readability". Thanks.

[facundocapua]

Hi @maksek , sorry for the lack of details, let me explain what are the changes about:

I've made 2 changes, both in the script for cache management via command line:

• Added 'status' command: The status of each cache type was being displayed at the end of every execution. I think it makes much more sense to have a command that let's you see the status as it is usually seen in this type of scripts.
  
  Actually, there is an 'info' command as part of the indexer.php script and also a 'status' command as part of the log.php script; so for me it was reasonable to have a 'status' command as part of the cache.php script instead of displaying the information every time the script is executed.
• The second change I've made is related to readability, for example, a flush command looks like this:

vagrant@packer-virtualbox-iso:/vagrant/htdocs/dev/shell$ php cache.php --flush
Flushed cache types: config, layout, block_html, view_files_fallback, view_files_preprocessing, collections, db_ddl, eav, full_page, translate, config_integration, config_webservice, config_integration_api
Current status:
                        config: 1
                        layout: 1
                    block_html: 1
           view_files_fallback: 1
      view_files_preprocessing: 1
                   collections: 1
                        db_ddl: 1
                           eav: 1
                     full_page: 1
                     translate: 1
            config_integration: 1
             config_webservice: 1
        config_integration_api: 1vagrant@packer-virtualbox-iso:/vagrant/htdocs/dev/shell$

As you can see all the cache flushed are in the same line separated with comma, which is hard to read. There is also a line break missing at the end of the output.

After the changes I've made, now this is the output for the flush:

vagrant@packer-virtualbox-iso:/vagrant/htdocs/dev/shell$ php cache.php --flush
Flushed cache types:
config
layout
block_html
view_files_fallback
view_files_preprocessing
collections
db_ddl
eav
full_page
translate
config_integration
config_webservice
config_integration_api
vagrant@packer-virtualbox-iso:/vagrant/htdocs/dev/shell$

The changes for readability affects all commands that list cache types (flush, clean and set -when some type of cache is enabled and gets flushed-).

Of course, I've updated the Unit tests scripts accordingly.

Let me know if this clarifies what the change are about. Thanks!

[verklov]

MAGETWO-32765

[maksek]

Thanks @fcapua-summa, very nice description. We are reviewing the PR.

[antonmakarenko]

As for me, the "status" change is detrimental for usability. Displaying status unconditionally is always more informative -- no matter change you made or even if you didn't perform any changes (just to see status). Why do you need a special command for it? What if you ran the command with no arguments? Displaying status is better than displaying nothing (sure, you can display a "help" in order to discover there is a "status" command -- but why have this extra step?).

Why is comma-separated line hard to read?

May I suggest these changes only:

• Improve readability of the "affected cache types" list by moving it to next line with an indent -- this way it is going to be emphasized, yet won't bloat half of your screen
• Fix for the missed trailing end of line

[facundocapua]

@antonmakarenko I mostly agree with what you are saying about the 'status' command. However, it is consistent with the other cli tools provided.

When running indexer.php alone, this is the output:

php indexer.php
Usage:  php -f indexer.php -- [options]

  --status <indexer>            Show Indexer(s) Status
  --mode <indexer>              Show Indexer(s) Index Mode
  --mode-realtime <indexer>     Set index mode type "Update on Save"
  --mode-schedule <indexer>     Set index mode type "Update by Schedule"
  --reindex <indexer>           Reindex Data
  info                          Show allowed indexers
  reindexall                    Reindex Data by all indexers
  help                          This help

  <indexer>     Comma separated indexer codes or value "all" for all indexers

When running dependency.php this is the output:

php dependency.php
Usage: php -f dependency.php -- [--list-modules] [--list-components][--list-component-dependencies component_name]
        [--list-component-dependents component_name] [--list-module-dependencies module-name]
        [--list module-dependents module-name] [--direct-dependency-only]
        --help - print usage message
        --list-modules - list all modules in order of module dependency
        --list-components - list all components consisting of circularly dependent modules
        --list-component-dependencies - list components that the specified component depends on
        --list-component-dependents - list components that depends on the specified components
        --list-module-dependencies - list modules that the specified module depends on
        --list-module-dependents - list modules that depends on the specified module
        --direct-dependency-only - only return direct dependencies

When running log.php, this is the output:

php log.php
Usage:  php -f log.php -- [options]
        php -f log.php -- clean --days 1

  clean             Clean Logs
  --days <days>     Save log, days. (Minimum 1 day, if defined - ignoring system value)
  status            Display statistics per log tables
  help              This help

I agree with the fact that to get the cache type status you have an extra step, but it is just the first time, once you know you won't need that extra step.

I think it also has to be with the practise of avoiding developers lose control of what they are doing. Why should I get the status when no instruction of doing that is given?

Regarding readability, it's not that is "really hard to read", you can easily read items separated by comma, but it has to be with the fact that the cache types are usually presented as a list; actually in the same cli tool when running any other command than flushing, the cache types are presented as a list.
I think is easier to find if you are looking for a particular cache type; and as shown in my previous comment, the status took way more presence rather than the list of cache types flushed in the "flush" command execution.

Let me know if this makes sense, or something is not clear enough.

[facundocapua]

I don't know why this PR was closed, I was trying to create a new PR for other issue and it appeared here as closed.

I must have messed something up. I created a new PR with the changes: #949

Sorry for the inconvenience.