.ASM files ignored?

[DecimalTurn]

I was looking at https://github.com/microsoft/GW-BASIC and it contains a lot of Assembly code (.ASM files).
However, when running github-linguist, it seems that those files are ignored. Am I missing something?

github-linguist output:

100.00% 12114      C
C:
BINTRP.H
IBMRES.H
OEM.H

output for specific file:

ADVGRP.ASM: 0 lines (0 sloc)
  type:      Binary
  mime type: text/x-asm
  language:  

Why is it detected as binary in the GW-BASIC repo and detected correctly in https://github.com/microsoft/MS-DOS?

EDIT: Turns out after opening the file in VS Code that there is a few NUL bytes at the end of the file.

Should Linguist ignore NULs at the end of a file?

[lildude]

You beat me to it. It's quite common for files that have come from Windows systems to be identified as binaries, but it's usually because of a byte order mark (BOM), which IIRC things like Visual Studio and other older dev tools add to files. This is a new one for me.

Should Linguist ignore NULs at the end of a file?

It's not up to Linguist. We use charlock_holmes to detect binaries:

linguist/lib/linguist/blob_helper.rb

Lines 122 to 142 in 39fd5e9

	# Public: Is the blob binary?
	#
	# Return true or false
	def binary?
	# Large blobs aren't even loaded into memory
	if data.nil?
	true
	# Treat blank files as text
	elsif data == ""
	false
	# Charlock doesn't know what to think
	elsif encoding.nil?
	true
	# If Charlock says its binary
	else
	detect_encoding[:type] == :binary
	end
	end

irb(main):001> require 'charlock_holmes'
=> true
irb(main):002> contents = File.read('ADVGRP.ASM')
=> "; [ This translation created 10-Feb-83 by Version 4.3 ]\r\n\r\n\t.RADIX  8\t\t; To be safe\r\n\r\nCSEG\tSEGMENT PUBLIC 'CODESG' \r\n\tASSUME  CS:CSEG\r\n\r\nINCLUDE\tOEM.H\r\n\r\n\tTITLE   ADVGRP - ADVANCE...
irb(main):003> CharlockHolmes::EncodingDetector.detect(contents)
=> {:type=>:binary, :confidence=>100}
irb(main):004>

[DecimalTurn]

Considering that the GitHub UI doesn't consider the file binary and neither does Go-Enry, is this worth opening an issue with charlock_holmes? Or could Linguist pre-emptively remove NULs at the end of the blob before passing it to charlock_holmes?

EDIT: The issue is here: brianmario/charlock_holmes#192

[lildude]

Considering that the GitHub UI doesn't consider the file binary and neither does Go-Enry, is this worth opening an issue with charlock_holmes?

Yes

Or could Linguist pre-emptively remove NULs at the end of the blob before passing it to charlock_holmes?

Possibly. We'd have to be mindful of the risk of stripping legit uses (I have no idea if there are any).

[DecimalTurn]

Perhaps we should ask directly if those NULs at the end of .ASM files for GW-BASIC are intentional first. @bitcrazed, could you please help us here?

[bitcrazed]

Sorry, but as stated in the GW-BASIC readme:

The source files in this repo are for historical reference and will remain read-only and unmodified in their original state. Please do not send Pull Requests suggesting any modifications to the source files.

[DecimalTurn]

@bitcrazed, thanks for the reply, but I was not asking to change the source code, just to understand if the presence NULs at the end was intentionally or just the result of some artifacts introduced at some point.

This is just to know if Linguist should stop considering NULs at the end of .ASM files as an indication that the file is binary since that's the reason why there's no Assembly in the language stats bar for the GW-BASIC repo:

[DecimalTurn]

I'm glad to see that the problem didn't repeat itself with BASIC-M6502.

@bitcrazed I just wanted mention to you that even without changing any of GW-BASIC's source code files, you could still fix the language detection with a simple .gitattributes file containing the following line:

*.ASM linguist-language=Assembly

Considering that a .gitignore file was already added to the repo, I don't see a reason against this approach, so hopefully that would be a valid approach.