[PATCH 0/2] btrfs-progs: completion: Small fixes to make debug simpler

[PATCH 0/2] btrfs-progs: completion: Small fixes to make debug simpler

[Qu Wenruo]

For developer, it's pretty common to use "btrfs check" or "btrfs ins
dump-tree" on raw dumps.

However "btrfs check" can only complete real block devices, and
"btrfs inspect dump-tree" can only complete dir.

Make them to use _filedir() so any filename can be completed and save us
developer a little time and nerve hitting that holy tab.

Qu Wenruo (2):
  btrfs-progs: completion: Use _filedir() to replace _btrfs_devs()
  btrfs-progs: completion: Let dump-tree/dump-super/inode-resolve to
    accept any file

 btrfs-completion | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

-- 
2.18.0

[PATCH 1/2] btrfs-progs: completion: Use _filedir() to replace _btrfs_devs()

[Qu Wenruo]

For developers it's pretty common to call "btrfs check" on a raw image
dump other than real block device.

So current _btrfs_devs() is really making things worse. Use _filedir()
to replace _btrfs_devs() so it can complete any filenames, no matter if
it's just a file or a real block device.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-completion | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/btrfs-completion b/btrfs-completion
index ae683f4ecf61..33830e827977 100644
--- a/btrfs-completion
+++ b/btrfs-completion
@@ -6,9 +6,7 @@
 
 _btrfs_devs()
 {
-	local DEVS
-	DEVS=''; while read dev; do DEVS+="$dev "; done < <(lsblk -pnro name)
-	COMPREPLY+=( $( compgen -W "$DEVS" -- "$cur" ) )
+	_filedir
 }
 
 _btrfs_mnts()
-- 
2.18.0

[PATCH 2/2] btrfs-progs: completion: Let dump-tree/dump-super/inode-resolve to accept any file

[Qu Wenruo]

For dump-tree/dump-super the completion uses default filedir -d, which
is far from convenient.
Use filedir for dump-tree/dump-super/inode-resolve just like rootid.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-completion | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/btrfs-completion b/btrfs-completion
index 33830e827977..b22a951b3a65 100644
--- a/btrfs-completion
+++ b/btrfs-completion
@@ -133,7 +133,7 @@ _btrfs()
 						_btrfs_mnts
 						return 0
 						;;
-					rootid)
+					dump-tree|dump-super|rootid|inode-resolve)
 						_filedir
 						return 0
 						;;
-- 
2.18.0

Re: [PATCH 0/2] btrfs-progs: completion: Small fixes to make debug simpler

[Misono Tomohiro]

This is useful.

Reviewed-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

On 2018/07/17 15:15, Qu Wenruo wrote:
> For developer, it's pretty common to use "btrfs check" or "btrfs ins
> dump-tree" on raw dumps.
> 
> However "btrfs check" can only complete real block devices, and
> "btrfs inspect dump-tree" can only complete dir.
> 
> Make them to use _filedir() so any filename can be completed and save us
> developer a little time and nerve hitting that holy tab.
> 
> Qu Wenruo (2):
>   btrfs-progs: completion: Use _filedir() to replace _btrfs_devs()
>   btrfs-progs: completion: Let dump-tree/dump-super/inode-resolve to
>     accept any file
> 
>  btrfs-completion | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 

Re: [PATCH 1/2] btrfs-progs: completion: Use _filedir() to replace _btrfs_devs()

[David Sterba]

On Tue, Jul 17, 2018 at 02:15:31PM +0800, Qu Wenruo wrote:
> For developers it's pretty common to call "btrfs check" on a raw image
> dump other than real block device.

I'd be less concerned about developers' environment rather than the
situations where users want to run check and won't accidentally screw
up. So completing from filenames should be safe here, the typical
use will probably want something starting with /dev/ and confusion with
random files named 'sda' is highly unlikely.

> So current _btrfs_devs() is really making things worse. Use _filedir()
> to replace _btrfs_devs() so it can complete any filenames, no matter if
> it's just a file or a real block device.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  btrfs-completion | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/btrfs-completion b/btrfs-completion
> index ae683f4ecf61..33830e827977 100644
> --- a/btrfs-completion
> +++ b/btrfs-completion
> @@ -6,9 +6,7 @@
>  
>  _btrfs_devs()
>  {
> -	local DEVS
> -	DEVS=''; while read dev; do DEVS+="$dev "; done < <(lsblk -pnro name)
> -	COMPREPLY+=( $( compgen -W "$DEVS" -- "$cur" ) )
> +	_filedir

This is not a good style, the function is says "devices" and actually
does "all files". It's used for several other commands where completing
files might not be a good idea.

If you want to use _filedir, then use it and don't obscure it.

Re: [PATCH 1/2] btrfs-progs: completion: Use _filedir() to replace _btrfs_devs()

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1946 bytes --]



On 2018年08月03日 22:00, David Sterba wrote:
> On Tue, Jul 17, 2018 at 02:15:31PM +0800, Qu Wenruo wrote:
>> For developers it's pretty common to call "btrfs check" on a raw image
>> dump other than real block device.
> 
> I'd be less concerned about developers' environment rather than the
> situations where users want to run check and won't accidentally screw
> up. So completing from filenames should be safe here, the typical
> use will probably want something starting with /dev/ and confusion with
> random files named 'sda' is highly unlikely.

Yep, _filedir() is just a superset of the original devices.
So for original user case, user only needs extra "/de" to initial the
completion and then it should work mostly fine.

Although it could be further enhanced to only complete certain types,
including regular file (for raw dump), block devices (end user use case).

> 
>> So current _btrfs_devs() is really making things worse. Use _filedir()
>> to replace _btrfs_devs() so it can complete any filenames, no matter if
>> it's just a file or a real block device.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  btrfs-completion | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/btrfs-completion b/btrfs-completion
>> index ae683f4ecf61..33830e827977 100644
>> --- a/btrfs-completion
>> +++ b/btrfs-completion
>> @@ -6,9 +6,7 @@
>>  
>>  _btrfs_devs()
>>  {
>> -	local DEVS
>> -	DEVS=''; while read dev; do DEVS+="$dev "; done < <(lsblk -pnro name)
>> -	COMPREPLY+=( $( compgen -W "$DEVS" -- "$cur" ) )
>> +	_filedir
> 
> This is not a good style, the function is says "devices" and actually
> does "all files". It's used for several other commands where completing
> files might not be a good idea.
> 
> If you want to use _filedir, then use it and don't obscure it.

Right, I'll just discard the _btrfs_devs() wrapper.

Thanks,
Qu

> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]