Enable --format=<format> for db search#247
Conversation
6d4b69d to
ab28315
Compare
ab28315 to
12cf8d6
Compare
12cf8d6 to
5a1d6e1
Compare
|
cc @johnbillion since you were involved in the discussion about this, maybe you'd like to take a look as well |
5a1d6e1 to
fd762a7
Compare
danielbachhuber
left a comment
There was a problem hiding this comment.
It'd be great to have a more detailed pull request description that describes what this pull request does, and how it changes the nature of the command.
src/DB_Command.php
Outdated
| * | ||
| * [--one_line] | ||
| * : Place the 'table:column' output on the same line as the row id and match ('table:column:id:match'). Overrides --table_column_once. | ||
| * : Deprecated: use `--format` instead. Place the 'table:column' output on the same line as the row id and match ('table:column:id:match'). Overrides --table_column_once. |
There was a problem hiding this comment.
What argument should they use with --format?
src/DB_Command.php
Outdated
| * | ||
| * [--matches_only] | ||
| * : Only output the string matches (including context). No 'table:column's or row ids are outputted. | ||
| * : Deprecated: use `--format` instead. Only output the string matches (including context). No 'table:column's or row ids are outputted. |
There was a problem hiding this comment.
What argument should they use with --format?
| * : Get a specific subset of the fields. | ||
| * | ||
| * [--format=<format>] | ||
| * : Render output in a particular format. |
There was a problem hiding this comment.
We should document supported --format=<format> options with YAML:
[--format=<format>]
---
options:
- TK
---
| * : Percent color code to use for the match (unless both before and after context are 0, when no color code is used). For a list of available percent color codes, see below. Default '%3%k' (black on a mustard background). | ||
| * | ||
| * [--fields=<fields>] | ||
| * : Get a specific subset of the fields. |
There was a problem hiding this comment.
What fields are supported here?
src/DB_Command.php
Outdated
| 'table' => $table, | ||
| 'column' => $column, | ||
| 'key' => $primary_key, | ||
| 'ID' => $result->$primary_key, |
There was a problem hiding this comment.
Should this be 'value' instead of 'ID'? It seems a little bit weird to call this 'ID' when the primary key column could be called something else.
|
@2ndkauboy Are you planning to continue working on this, or should we close the PR for now? |
|
@danielbachhuber i was hoping to get feedback from @johnbillion about this new feature, since he opened the issue. |
|
I'll take a look! |
|
@2ndkauboy I agree that introducing a |
So you would also rename the |
|
Would people understand that |
|
What's the command for that output? |
|
The command for the output above would be: |
|
I think Perhaps the order of the columns should change to: If we didn't need to support output from multiple database tables we could skip the |
By default, the So this includes: table, column, primary key value, match. This is why decided to have all these columns as well. But then the "primary key value" column needed a headline/name, and since it could be different column names, I added that If we allow searching all tables, which the command does by default, we cannot use the "primary key name" as the headline/name, since it's different names. We could just not have that column, but then we need another headline. Maybe something like |
|
Yeah. Perhaps |
501ffd9 to
beccc58
Compare
beccc58 to
1c9b2e8
Compare
|
I thought about that as well. How about |
|
@johnbillion I've simulated some variant, which one would you pick? Only the primary key value1. Column
|
|
Let's go with this: |
b65a6dd to
05263c3
Compare
|
The result would look like this now: |
|
@2ndkauboy I think that looks good. What do you think? |
05263c3 to
9e9ec17
Compare
9e9ec17 to
531b897
Compare
|
Let's merge it then :) After my updated tests run ;) |
--format=<format> for db search
danielbachhuber
left a comment
There was a problem hiding this comment.
Thanks for your work on this, @2ndkauboy !
|
Updated at WP-CLI Hack Day April 2024 #5935 |
Closes #158
Related wp-cli/wp-cli#5859