Align tables in "ls -f" (#2105)

* Stuff column with nothing if we have nothing

* Stuff columns at the very start

Remove unnecessary else clauses.
Add the unix cfg portion

* Added some tests and cfg windows

Not sure how I feel about these tests but it's better than nothing

crates/nu-cli/src/data/files.rs

@@ -45,6 +45,38 @@ pub(crate) fn dir_entry_dict(
 ) -> Result<Value, ShellError> {
     let tag = tag.into();
     let mut dict = TaggedDictBuilder::new(&tag);
+    // Insert all columns first to maintain proper table alignment if we can't find (or are not allowed to view) any information
+    if full {
+        #[cfg(windows)]
+        {
+            for column in [
+                "name", "type", "target", "readonly", "size", "created", "accessed", "modified",
+            ]
+            .iter()
+            {
+                dict.insert_untagged(*column, UntaggedValue::nothing());
+            }
+        }
+
+        #[cfg(unix)]
+        {
+            for column in [
+                "name", "type", "target", "readonly", "mode", "uid", "group", "size", "created",
+                "accessed", "modified",
+            ]
+            .iter()
+            {
+                dict.insert_untagged(&(*column.to_owned()), UntaggedValue::nothing());
+            }
+        }
+    } else {
+        for column in ["name", "type", "target", "size", "modified"].iter() {
+            if *column == "target" && !with_symlink_targets {
+                continue;
+            }
+            dict.insert_untagged(*column, UntaggedValue::nothing());
+        }
+    }
 
     let name = if short_name {
         filename.file_name().and_then(|s| s.to_str())
@@ -63,15 +95,12 @@ pub(crate) fn dir_entry_dict(
 
     if let Some(md) = metadata {
         dict.insert_untagged("type", get_file_type(md));
-    } else {
-        dict.insert_untagged("type", UntaggedValue::nothing());
     }
 
     if full || with_symlink_targets {
         if let Some(md) = metadata {
-            let mut symlink_target_untagged_value: UntaggedValue = UntaggedValue::nothing();
-
             if md.file_type().is_symlink() {
+                let symlink_target_untagged_value: UntaggedValue;
                 if let Ok(path_to_link) = filename.read_link() {
                     symlink_target_untagged_value =
                         UntaggedValue::string(path_to_link.to_string_lossy());
@@ -79,9 +108,8 @@ pub(crate) fn dir_entry_dict(
                     symlink_target_untagged_value =
                         UntaggedValue::string("Could not obtain target file's path");
                 }
+                dict.insert_untagged("target", symlink_target_untagged_value);
             }
-
-            dict.insert_untagged("target", symlink_target_untagged_value);
         }
     }
 
@@ -116,13 +144,6 @@ pub(crate) fn dir_entry_dict(
                     );
                 }
             }
-        } else {
-            dict.insert_untagged("readonly", UntaggedValue::nothing());
-
-            #[cfg(unix)]
-            {
-                dict.insert_untagged("mode", UntaggedValue::nothing());
-            }
         }
     }
 
@@ -157,8 +178,6 @@ pub(crate) fn dir_entry_dict(
         }
 
         dict.insert_untagged("size", size_untagged_value);
-    } else {
-        dict.insert_untagged("size", UntaggedValue::nothing());
     }
 
     if let Some(md) = metadata {
@@ -175,13 +194,6 @@ pub(crate) fn dir_entry_dict(
         if let Ok(m) = md.modified() {
             dict.insert_untagged("modified", UntaggedValue::system_date(m));
         }
-    } else {
-        if full {
-            dict.insert_untagged("created", UntaggedValue::nothing());
-            dict.insert_untagged("accessed", UntaggedValue::nothing());
-        }
-
-        dict.insert_untagged("modified", UntaggedValue::nothing());
     }
 
     Ok(dict.into_value())

crates/nu-cli/tests/commands/ls.rs

@@ -1,5 +1,5 @@
 use nu_test_support::fs::Stub::EmptyFile;
-use nu_test_support::playground::Playground;
+use nu_test_support::playground::{Dirs, Playground};
 use nu_test_support::{nu, pipeline};
 
 #[test]
@@ -154,3 +154,60 @@ fn list_files_from_two_parents_up_using_multiple_dots() {
         assert_eq!(actual.out, "5");
     })
 }
+
+#[test]
+fn list_all_columns() {
+    Playground::setup(
+        "ls_test_all_columns",
+        |dirs: Dirs, sandbox: &mut Playground| {
+            sandbox.with_files(vec![
+                EmptyFile("Leonardo.yaml"),
+                EmptyFile("Raphael.json"),
+                EmptyFile("Donatello.xml"),
+                EmptyFile("Michelangelo.txt"),
+            ]);
+            // Normal Operation
+            let actual = nu!(
+                cwd: dirs.test(),
+                "ls | get | to md"
+            );
+            let expected = ["name", "type", "size", "modified"].join("");
+            assert_eq!(actual.out, expected, "column names are incorrect for ls");
+            // Symbolic Links
+            let actual = nu!(
+                cwd: dirs.test(),
+                "ls -w | get | to md"
+            );
+            let expected = ["name", "type", "target", "size", "modified"].join("");
+            assert_eq!(actual.out, expected, "column names are incorrect for ls -w");
+            // Full
+            let actual = nu!(
+                cwd: dirs.test(),
+                "ls -f | get | to md"
+            );
+            let expected = {
+                #[cfg(unix)]
+                {
+                    [
+                        "name", "type", "target", "readonly", "mode", "uid", "group", "size",
+                        "created", "accessed", "modified",
+                    ]
+                    .join("")
+                }
+
+                #[cfg(windows)]
+                {
+                    [
+                        "name", "type", "target", "readonly", "size", "created", "accessed",
+                        "modified",
+                    ]
+                    .join("")
+                }
+            };
+            assert_eq!(
+                actual.out, expected,
+                "column names are incorrect for ls full"
+            );
+        },
+    );
+}