(makefile) Syntax highlighting of Makefiles

[TriMoon]

trafficstars

Describe the issue Syntax highlighting of Makefiles is bugged.

Which language seems to have the issue? lang-makefile

Are you using highlight or highlightAuto? no idea what Stackexchange uses...

Sample Code to Reproduce

• demo

#	SPDX-License-Identifier: CC-BY-NC-SA-4.0
#
SHELL = /usr/bin/bash
.SHELLFLAGS = -ec
.ONESHELL:
.SILENT:

UTILS			:=
UTILS			+= setfacl
UTILS			+= chown
UTILS			+= chmod
# UTILS			+= nonexisting

$(info ### START: Utils check)
# Define macro wrt shell.
ifeq (bash,$(findstring bash,$(SHELL)))
toUpper	= $(shell string="$(strip $1)"; printf "%s" $${string^^})
else
# Insipred by: https://stackoverflow.com/a/37660916/22510042
toUpper = $(shell echo "$(strip $1)" | sed 's/.*/\U&/')
endif
# Inspired by: https://stackoverflow.com/a/37197276/22510042
$(foreach util,shell $(UTILS),\
	$(if $(filter-out shell,$(util)),\
		$(if $(shell command -v $(util) 2> /dev/null),\
			$(eval \
				$(shell \
					printf "%s\t:=\t%s\n" \
						$(call toUpper,$(util) ) \
						$(shell command -v $(util) 2> /dev/null)\
				)\
			)\
			,$(error No '$(util)' in PATH)\
		)\
	)\
	$(info $(call toUpper,$(util))	= $($(call toUpper,$(util))))\
)
# There is a \t char before the = in the info line above for alignment...
$(info ### END: Utils check)
$(info ) # Empty line
# $(error END: Forced)

Expected behavior

Something similar to how kate renders it, although that has some errors also... (see the orange lines)

Additional context

See the Meta discussion: https://meta.stackexchange.com/q/393515/1288919

[joshgoebel]

Well the "all green" after a certain point is likely a bug in how we handle strings, but if that sample is any indicator it looks like our handling of makefiles is pretty poor. Would definitely be open to someone stepping up and improving things here. The grammar is currently pretty simple and should be easier to build on than some.

[aneesh98]

Hi Would like to work on this issue

[joshgoebel]

Go for it. :)

[aneesh98]

Hi I am still working on the fix for this issue, and I noticed that the "dockerfile.js" present "src/languages" is being interpreted as a Dockerfile in VSCode (which I am using for development). Leading to incorrect syntax highlighting. Check below screenshot for reference.

I observed this also happens if I make files with any other extension (e.g. .py files for python). While this has got nothing to do with the highlight.js. I just wanted to open an issue in VSCode, and cite this file from the project as example where the issue occurs in VSCode. Hope that's fine?

[joshgoebel]

All our code is open source, share it wherever... But I think one docker convention is that Dockerfile.* can be a dockerfile... so that might technically be correct in some sense, despite also being very wrong. Curious what the VS Code people will say.

[aneesh98]

HI @joshgoebel . I have made the following change, Currently the highlighting issue starts from the definition of toUpper macro within the ifeq statement:

ifeq (bash,$(findstring bash,$(SHELL)))
toUpper	= $(shell string="$(strip $1)"; printf "%s" $${string^^})

I believe this is happening because the first string mentioned in the function call '"$(strip $1)"' is not being identified correctly and I think the cause is 'FUNC' variable in makefile.js doesn't currently include QUOTE_STRING

https://github.com/highlightjs/highlight.js/blob/17494c81dbade12834feedddf6fb208bcc9cbcce/src/languages/makefile.js#L32-L42

If I change the same to include QUOTE_STRING as done in my PR

https://github.com/aneesh98/highlight.js/blob/904ddd1295e48c1097795144b8bf21a533a8c6cf/src/languages/makefile.js#L32-L45

This is the obtained result.

This does solve the initial formatting issue. But I observe few things here. The string "$(strip $1)" is a function call is not being highlighted as per function call rules of Makefile but is being highlighted as a plain string, as opposed to the screenshot posted under "expected behaviour" for this issue.

I am trying to fix this, but unable to understand on how I can proceed :sweat_smile: . Can you please help? Another few things I observe is function "filter-out" has two formatting one before the hyphen and one after the hyphen could this be a RegExp issue?.

Kindly help. Thanks. :smile:

[joshgoebel]

Keywords match in the order defined, so reverse the order of filter and filter-out.

If we're going to start including more things in $() then it's no longer only variables we're dealing with so we should remove that top-level scope... perhaps add another rule to detect only SIMPLE variables like $(name_here)...

I'm not sure exactly what the context of the code inside the `$() is though vs that outside.

[TriMoon]

@aneesh98

But I observe few things here. The string "$(strip $1)" is a function call is not being highlighted as per function call rules of Makefile but is being highlighted as a plain string

Maybe that's because of: https://github.com/highlightjs/highlight.js/blob/17494c81dbade12834feedddf6fb208bcc9cbcce/src/languages/makefile.js#L11-L12 https://github.com/highlightjs/highlight.js/blob/17494c81dbade12834feedddf6fb208bcc9cbcce/src/languages/makefile.js#L32-L33

I am trying to fix this, but unable to understand on how I can proceed 😅 . Can you please help?

hence the value of className needs to be different? :thinking: :wink: https://github.com/highlightjs/highlight.js/blob/main/docs/css-classes-reference.rst

[joshgoebel]

Correct or blank, but I'm not familiar enough with Makefile to know what that content is...

[TriMoon]

• "$(strip $1)" = The result is a string.
• $(strip $1) = Is a function call.
• strip = the function name.
• $1 = the argument to the function. Multiple arguments are separated by a comma if that function defines that... In the case of the shell function they are separated by space like in a normal invocation of a (bash/sh) shell function.

[aneesh98]

Keywords match in the order defined, so reverse the order of filter and filter-out.

If we're going to start including more things in $() then it's no longer only variables we're dealing with so we should remove that top-level scope... perhaps add another rule to detect only SIMPLE variables like $(name_here)...

I'm not sure exactly what the context of the code inside the `$() is though vs that outside.

I understood for filter and filter-out, thanks for the explanation. I will make the change. Little confused about what you said after that :sweat_smile:

Is it regarding having function call and multiple arguments in $() like $(foreach util,shell $(UTILS)

What do you mean by top-level scope here?

If we're going to start including more things in $() then it's no longer only variables we're dealing with so we should remove that top-level scope

[aneesh98]

@aneesh98

But I observe few things here. The string "$(strip $1)" is a function call is not being highlighted as per function call rules of Makefile but is being highlighted as a plain string

Maybe that's because of:

https://github.com/highlightjs/highlight.js/blob/17494c81dbade12834feedddf6fb208bcc9cbcce/src/languages/makefile.js#L11-L12

https://github.com/highlightjs/highlight.js/blob/17494c81dbade12834feedddf6fb208bcc9cbcce/src/languages/makefile.js#L32-L33

I am trying to fix this, but unable to understand on how I can proceed 😅 . Can you please help?

hence the value of className needs to be different? 🤔 😉 https://github.com/highlightjs/highlight.js/blob/main/docs/css-classes-reference.rst

Ahh yess. Guess the FUNC variable needs a class name change, will look into this. Thanks

[TriMoon]

FYI: The current changes in #3898 are not adequate to fix the problem in the OP. So if others want to help or take-over the work of @aneesh98 feel free :wink:

[!NOTE] To assist you guys in improving the highlighting of makefiles: The latest version of the main functionality of this Makefile is available in one-of-my repo's at GitLab. 😉🤝 (Who knows maybe it will serve you also on occasions)