Nushell: `ls | where size > 1kb` no longer works

Created on 19 Mar 2020  路  3Comments  路  Source: nushell/nushell

Describe the bug
In directories that contain other directories, you can no longer do ls | where size > 1kb. It will error because the directory size is blank and treated as not compatible with the comparison.

To Reproduce
Steps to reproduce the behavior:

  1. Have a directory that contains other directories
  2. Run ls | where size > 1kb
  3. 3.

Expected behavior
The nothing should be treated as 0

Screenshots

image

Configuration (please complete the following information):

  • OS: Windows
  • Version 0.11.1
bug polish

Most helpful comment

This was introduced in commit c43a58d9d677e3f35e46bf2f5fc566dc8d1aff5d.
@thegedge the metada from std already contains the 4KB info so it's a on line patch :

diff --git a/crates/nu-cli/src/data/files.rs b/crates/nu-cli/src/data/files.rs
index 95cbcfd..34461cd 100644
--- a/crates/nu-cli/src/data/files.rs
+++ b/crates/nu-cli/src/data/files.rs
@@ -128,7 +128,7 @@ pub(crate) fn dir_entry_dict(
     }

     if let Some(md) = metadata {
-        if md.is_file() {
+        if md.is_file() || md.is_dir() {
             dict.insert_untagged("size", UntaggedValue::bytes(md.len() as u64));
         } else {
             dict.insert_untagged("size", UntaggedValue::nothing());

However I think that the best way to do it from the point of view of the user would be to display nothing (or 4KB) and ignore directories when using ls | where something since the 4KB size is rarely what we're after in that use case.

Using the total size of the directory would be the best but it would be way too slow depending on the directory structure and inaccurate when permissions to access some sub-directories are lacking.
Maybe a flag could be passed by the user when this is the desired behavior (though du is probably already enough)

All 3 comments

The nothing should be treated as 0

Another option is to show the size a directory entry takes up on the disk. This is typically 1 block (4k). ls typically does this 馃檪

This was introduced in commit c43a58d9d677e3f35e46bf2f5fc566dc8d1aff5d.
@thegedge the metada from std already contains the 4KB info so it's a on line patch :

diff --git a/crates/nu-cli/src/data/files.rs b/crates/nu-cli/src/data/files.rs
index 95cbcfd..34461cd 100644
--- a/crates/nu-cli/src/data/files.rs
+++ b/crates/nu-cli/src/data/files.rs
@@ -128,7 +128,7 @@ pub(crate) fn dir_entry_dict(
     }

     if let Some(md) = metadata {
-        if md.is_file() {
+        if md.is_file() || md.is_dir() {
             dict.insert_untagged("size", UntaggedValue::bytes(md.len() as u64));
         } else {
             dict.insert_untagged("size", UntaggedValue::nothing());

However I think that the best way to do it from the point of view of the user would be to display nothing (or 4KB) and ignore directories when using ls | where something since the 4KB size is rarely what we're after in that use case.

Using the total size of the directory would be the best but it would be way too slow depending on the directory structure and inaccurate when permissions to access some sub-directories are lacking.
Maybe a flag could be passed by the user when this is the desired behavior (though du is probably already enough)

One approach we could try is the one I did with https://github.com/nushell/nushell/pull/1508, which keeps the view the same, and keeps using nothing for directories.

To account for this, we allow numeric comparison to work against nothing and treat it as a zero. If the value is totally missing, we still error, but this allows us to treat nothing as a valid number in this case.

Happy to chat about alternate approaches, this is just the one that came to mind this morning.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

alper picture alper  路  4Comments

ChrisDenton picture ChrisDenton  路  4Comments

twe4ked picture twe4ked  路  4Comments

ProgrammingLife picture ProgrammingLife  路  3Comments

ne-on picture ne-on  路  5Comments