Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: Flag to report missing newline before EOF #975

Open
lorenzleutgeb opened this issue May 4, 2020 · 14 comments
Open

Request: Flag to report missing newline before EOF #975

lorenzleutgeb opened this issue May 4, 2020 · 14 comments
Labels
feature-request New feature or request

Comments

@lorenzleutgeb
Copy link

lorenzleutgeb commented May 4, 2020

Hi!

I am a zsh user and got used to my shell "reporting" when there is no newline at the end of the output (a "partial" line):

$ /usr/bin/cat id
sha256:197a075f86ebf05daa4509d81e07d6c153443ae0cc5539bf8d82ae1b11ef64a6%

(Note the %.)

However, that doesn't work with bat.

$ bat id
   1   sha256:197a075f86ebf05daa4509d81e07d6c153443ae0cc5539bf8d82ae1b11ef64a6
$ bat --plain id
sha256:197a075f86ebf05daa4509d81e07d6c153443ae0cc5539bf8d82ae1b11ef64a6
$ bat --version
bat 0.11.0

I'd like to go one step further, and request a flag on bat that will print an explicit warning in case the input does not end in a newline character. Such as:

$ bat --warn-on-missing-newline id
   1   sha256:197a075f86ebf05daa4509d81e07d6c153443ae0cc5539bf8d82ae1b11ef64a6
   ⚠   No newline at end of file

Edit after comment:

I agree that it might be bad to use an emoji, so I propose:

$ bat --warn-on-missing-newline id
 1 | sha256:197a075f86ebf05daa4509d81e07d6c153443ae0cc5539bf8d82ae1b11ef64a6
 ! | No newline at end of file!
@lorenzleutgeb lorenzleutgeb added the feature-request New feature or request label May 4, 2020
@eth-p
Copy link
Collaborator

eth-p commented May 5, 2020

I like this idea, but I'm not that much of a fan of using an emoji inside the output. There's still a few environments that can't render them, sadly.

We would also need to consider what it looks like with the grid mode. Maybe something like this?

  12 | Hello world
 <!> | [Missing Newline]

@lorenzleutgeb
Copy link
Author

Sure, that was just what came to mind as an example. We're down to a matter of taste, but I'd prefer ! (without angle brackets) over <!> (with angle brackets). Please go ahead and choose as you like!

@eth-p
Copy link
Collaborator

eth-p commented May 6, 2020

Perhaps [!] in yellow?

As for the command line options, after a little thought, maybe it should be something similar to how we handle style components:

--warning=missing-trailing-newline,...

That could be added to the config file, and it would be more flexible for if we ever added more warning types (e.g. mixed indentation maybe?).

@lorenzleutgeb
Copy link
Author

Just FYI, git diff says "No newline at end of file"

@sharkdp
Copy link
Owner

sharkdp commented May 10, 2020

Thank you for reporting this.

The fact that bat -p prints a trailing newline (even if there is no newline at the end of the file) should be considered a bug, I believe (but we should check older tickets, if there was a reason for this behavior). Note that it works as expected in non-interactive mode, i.e. if the output is piped somewhere else:

printf "hello" | bat -p | hexdump -C
00000000  68 65 6c 6c 6f                                    |hello|
00000005

# force "interactive" mode:printf "hello" | bat -p --decorations=always | hexdump -C
00000000  68 65 6c 6c 6f 0a                                 |hello.|
00000006

I'd like to go one step further, and request a flag on bat that will print an explicit warning in case the input does not end in a newline character.

I'm not sure if I'm a big fan of this feature. Yes, the perfectionist in me also doesn't like files without newlines at the end, but do we really need bat to print a warning in this case? What is an actual use case where this would be useful?

Note that we already have -A/--show-all to make whitespace visible:

❯ bat -A with-newline without-newline
───────┬───────────────────────────────────────────
       │ File: with-newline
───────┼───────────────────────────────────────────
   1   │ hello␊
───────┴───────────────────────────────────────────
───────┬───────────────────────────────────────────
       │ File: without-newline
───────┼───────────────────────────────────────────
   1   │ hello
───────┴───────────────────────────────────────────

@obfusk
Copy link

obfusk commented May 27, 2020

I'm not sure if I'm a big fan of this feature. Yes, the perfectionist in me also doesn't like files without newlines at the end, but do we really need bat to print a warning in this case? What is an actual use case where this would be useful?

I've run into some "fun" bugs caused by (sublime text and other) editors not ending the last line with a newline (as required by POSIX). For example, wc -l will not count the last line, and a while read line; do ...; done loop in bash will not process the last line. Having tools like bat warn about this (like git does) is IMHO not just a "perfectionist" issue.

@mouse07410
Copy link

"Normal" people don't need this, but occasionally it may turn out useful. Normally I don't want this warning, but sometimes I might.

Having that warning controlled by an explicit flag seems a good idea.

@sharkdp
Copy link
Owner

sharkdp commented May 27, 2020

I've run into some "fun" bugs caused by (sublime text and other) editors not ending the last line with a newline (as required by POSIX). For example, wc -l will not count the last line, and a while read line; do ...; done loop in bash will not process the last line. Having tools like bat warn about this (like git does) is IMHO not just a "perfectionist" issue.

@obfusk Thank you for the feedback!

Indeed:

$ printf "1\n2\n3" | (while read line; do echo $line; done)
1
2
$ printf "1\n2\n3" | wc -l
2

I didn't know that. I also didn't know that POSIX requires an EOL at the EOF.

So maybe we could choose something less intrusive (instead of printing an additional line if there is no EOL)? Maybe we could print a red sign at the end of the last line? Or show that warning sign somewhere within the grid or the sidebar?

@obfusk
Copy link

obfusk commented May 27, 2020

I didn't know that. I also didn't know that POSIX requires an EOL at the EOF.

POSIX defines a line as a sequence of zero or more non- <newline> characters plus a terminating <newline> character. No special case for EOF. Assuming that it's a separator/delimiter instead is quite understandable, but -- unfortunately -- wrong.

@obfusk
Copy link

obfusk commented May 27, 2020

I agree that having a big warning is probably overkill. But I would very much recommend having some kind of warning by default. And an option to disable it for those who know what they are doing.

@eth-p
Copy link
Collaborator

eth-p commented May 28, 2020

I'm not sure who's going to end up implementing this, but I would recommend that it's done similarly to the style components. If we ever have another warning to add, I think it would be better to have a single flag for them all (e.g. --warning=something,another instead of --warn-on-something and --warn-on-another).

@lorenzleutgeb
Copy link
Author

Sorry that I didn't mention the POSIX definition of a line in my initial post, but that's exactly where my troubles and this request is rooted. Thanks to @obfusk for bringing in the convincing arguments.

@obfusk
Copy link

obfusk commented May 31, 2020

Note that wc -l by definition counts the number of <newline> characters in a file. Which results in printf "1\n2\n3" | wc -l == 2.

Whilst bat is of course not responsible for the creation of these "broken" files, it should IMHO never hide that fact from the user as it does now.

FYI: git says No newline at end of file (as mentioned), vim says noeol, cat at least doesn't insert a nonexistent newline.

@obfusk
Copy link

obfusk commented May 31, 2020

So maybe we could choose something less intrusive (instead of printing an additional line if there is no EOL)? Maybe we could print a red sign at the end of the last line? Or show that warning sign somewhere within the grid or the sidebar?

If you use a warning sign instead of a message, please make it easy for users to find out what it means.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants