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

Slight bug in fstab syntax definition? #2246

Closed
H4CKY54CK opened this issue Jul 18, 2022 · 2 comments
Closed

Slight bug in fstab syntax definition? #2246

H4CKY54CK opened this issue Jul 18, 2022 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers syntax-highlighting

Comments

@H4CKY54CK
Copy link

H4CKY54CK commented Jul 18, 2022

At the risk of doing everything completely incorrectly, I have replaced the bug report template with something more concise and helpful. I also did a quick search through these issues for "fstab" before making this bug report. Correction, I looked through open issues for fstab. I just checked the closed ones, and it looks like this was actually noticed in the initial addition of the syntax definition, but may have just flew under the radar by accident. #706 (comment)

In the syntax definition for fstab, it should also accept a 2 as a valid option for the 6th field. I only noticed because I was trying to use the syntax file in Sublime Text and saw the big red block indicating a syntax error. In the output of bat /etc/fstab, that 2 is just white, which looked correct. Here's a permalink to the line in question.

EDIT: I also meant to link to that original mention.

@H4CKY54CK H4CKY54CK added the bug Something isn't working label Jul 18, 2022
yuvalmo added a commit to yuvalmo/bat that referenced this issue Jul 23, 2022
Was missing the number 2 as a valid option in those fields.
@yuvalmo
Copy link
Contributor

yuvalmo commented Jul 23, 2022

I would love to fix this issue. I noticed the same fix is needed for the fstab_pass field as well:


I also edited the fstab regression test so it covers all the possible range of numbers for these fields, and made sure it works correctly.

You can review the fix here: #2250.

P.S. I'm pretty new to open-source, so I hope I didn't miss anything in the process. Please let me know if I did and I'll fix it.

@keith-hall
Copy link
Collaborator

Thanks for your contribution @yuvalmo, looks good to me! :)

sharkdp pushed a commit that referenced this issue Jul 25, 2022
Was missing the number 2 as a valid option in those fields.
@sharkdp sharkdp closed this as completed Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers syntax-highlighting
Projects
None yet
Development

No branches or pull requests

4 participants