Messy output when trying to show a binary file

[chshersh]

The entire layout just goes horribly wrong.

Solution:

1. Check that the file is binary.
2. Don't call bat if the file is binary (happens here)

   • github-tui/lib/fs/fs.ml

     Lines 32 to 37 in b2c44c1

     	let read_file_contents path =
     	let cmd =
     	"bat --style=numbers,changes --color=always --italic-text=always \
     	--paging=never --terminal-width=80 " ^ path
     	in
     	let contents = Shell.proc_stdout cmd in

3. Instead, display some standard text saying that the file is binary
4. Additionally, use the eae8 icon for the file icon when the file is binary

• 

[chshersh]

All points from this issue except the last one (pretty binary file icon) are done by @heathhenley in:

• Check for binary file warning and don't print #11

I'll keep the issue open till then.

[chshersh]

Hey, @heathhenley, btw, let me know if you want to finish this issue 🙂

I'm happy to discuss possible implementation and share my thoughts in advance, if you're still interested!

[heathhenley]

I'm in, happy to hear your suggestions in advance - that would be great!

I actually was planning to do it - but when I started to look at it I noticed that I don't have the little file/folder icons loading, so I think I've got to get nerd font installed in my wsl first before trying to add the new icon. I'll sort that out and then take a crack at your suggestions

EDIT: font installed 🎉

[heathhenley]

I see where it's deciding which icon to show, it's straightforward to add another match to use the binary icon instead. What I'm not sure about is how you would want to go about checking here? It probably doesn't make sense to read the actual file contents as we do in the other check - maybe match the filename against a list of file extensions that are likely binary?

[chshersh]

Happy to see you've figured out the font!

In the future, it's probably a good idea to check for the supported font and use some alternative simpler symbols, so the UI doesn't break when the font is not installed. Feel free to create an issue about that to track that item 🙂

---

As for this issue, I think I'd add a new enumeration field to the File constructor with types like type file_type = Text | Binary

github-tui/lib/fs/fs.ml

Line 7 in 71fa359

| File of string * file_contents lazy_t

And then here it becomes possible to pattern match on that value and add the corresponding icon as a prefix:

github-tui/lib/tui/widget.ml

Line 104 in 71fa359

| File (name, _) -> file_char ^ " " ^ pad name

[chshersh]

It probably doesn't make sense to read the actual file contents as we do in the other check - maybe match the filename against a list of file extensions that are likely binary?

So, since you've already implemented the check, I propose to set this Binary file type here but looks like some code will need to be refactored in a minor way:

github-tui/lib/fs/fs.ml

Lines 44 to 70 in 71fa359

	let read_file_contents path =
	let cmd =
	"bat --style=numbers,changes --color=always --italic-text=always \
	--paging=never --terminal-width=80 " ^ path
	in
	let contents = Shell.proc_stdout cmd in
	if has_binary_warning contents then
	{ lines = [| Pretty.str binary_file_warning |]; offset = 0 }
	else
	let lines =
	contents
	|> String.split_on_char '\n'
	|> List.map Pretty.str
	|> Array.of_list
	in
	{ lines; offset = 0 }
	let rec to_tree path =
	if Sys.is_directory path then
	let children =
	Array.map
	(fun child_name -> to_tree (Filename.concat path child_name))
	(Sys.readdir path)
	in
	let dirname = Filename.basename path in
	Dir (dirname, children)
	else File (Filename.basename path, lazy (read_file_contents path))