| Message ID | 20180717061532.11857-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >
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()
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(-)