Bug 144142 - kwrite syntax highlighting for makefiles: doesn't highlight error of spaces vs tabs
Summary: kwrite syntax highlighting for makefiles: doesn't highlight error of spaces v...
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: syntax (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-12 16:04 UTC by Richard Neill
Modified: 2013-10-09 15:09 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
katepart syntax highlighting for makefiles: mark leading spaces at every line as error (844 bytes, patch)
2012-05-09 20:45 UTC, Andreas Nordal
Details
syntax file for Makefile: marks leading spaces in rule definitions as errors (9.53 KB, application/xml)
2012-05-15 12:08 UTC, Andreas Nordal
Details
old syntax file for Makefile (6.28 KB, application/xml)
2012-05-15 12:21 UTC, Andreas Nordal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Neill 2007-04-12 16:04:48 UTC
Version:            (using KDE KDE 3.5.6)
Installed from:    Ubuntu Packages
OS:                Linux

The most common mistake in writing makefiles is to have leading spaces rather than tabs. However, kwrite does not highlight this. 

Suggestion: if file type == Makefile, then highlight leading spaces with a background colour.
Comment 1 Jon Nelson 2009-12-08 18:53:09 UTC
There is already "show tabulators" type of option.
I feel that the kate *variables* section for makefiles should contain (at least) 

kate: space-indent off; mixedindent off;

because spaces in makefiles are rarely (if ever) appropriate as indenting.

Tools -> Configure -> Editor Component -> Open/Save -> Modes & Filetypes

Choose Other/Makefile

Place 

kate: space-indent off; mixedindent off;

into the Variables section.  Now Makefiles automatically get tab-based indents even if you normally use space-based indenting.

Can this be added to the next update for the kate highlighting files?
Comment 2 Dominik Haumann 2009-12-08 22:05:08 UTC
> Can this be added to the next update for the kate highlighting files?

Already added and in KDE 4.3.4 + KDE 4.4, see bug #163220:
Variables=kate: tab-width 8; indent-width 8; replace-tabs off; replace-tabs-save off;

Highlighting leading spaces of course can be done, i.e. is a valid wish.
Comment 3 Andreas Nordal 2012-05-09 20:45:22 UTC
Created attachment 70987 [details]
katepart syntax highlighting for makefiles: mark leading spaces at every line as error

This is not a valid solution, but may work for some.

Problem is, leading spaces are actually valid, and used in some places, e.g:
ifndef PREFIX
  PREFIX=/usr/local
endif

This patch will mark those spaces too!
But one can use tabs wherever spaces are allowed anyway.

A proper solution would require expanding the state machine to work out if we are in a rule definition or not, and only check for leading spaces there. I actually tried that. Unfortunately, my solution only worked for every other rule, because I don't know how to backtrack, n00b as I am. As a bonus, expanding the state machine avoided checking for keywords in targets, prerequisites and rule definitions, where they should be ignored.

So instead of my bloated attempt that only worked partially, here is the trivial patch that works, but is a bit wrong.
Comment 4 Andreas Nordal 2012-05-15 12:08:08 UTC
Created attachment 71104 [details]
syntax file for Makefile: marks leading spaces in rule definitions as errors

This is a proper fix.

Sorry for fixing many things at once, but I did.
(And I didn't keep the intermediate versions.)

You see, while attempting to make the requested fix,
I could not overlook a line in my makefile that looked totally braindamaged.
Summary of how that line is highlighted differently now (this is going to look garbled in non-monospace):

override MAKEDIRS:=lib/ $(filter-out lib/, $(dir $(wildcard */Makefile)))
^^^^^^^^         ^^       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^
   |            /         |              |        |         |   /      |
Add the    Mark          Don't mistreat  Color    | ,-> Function       |
"override" assignment    function calls  commas   |/    parameters     |
keyword    operators as  as variables             |     are strings    |
           such, not as  in assignments          /|                    |
           strings       -Consequently ---------´ `---------.----------´
                          |                     \           |
                         Fix "filter-out"        `----> Fix nesting


There are mainly 2 things I have refactored from the 1.12 version I started with
plus some random fixes.

Main thing 1:
Expand the state machine to identify prerequisites and rule definitions.
This allowed me to:
* mark leading spaces as errors in rule definitions (Finally!)
* don't recognise keywords in rule definitions
* don't recognise keywords, singlequotes or doublequotes in prerequisites
* color prerequisites (default color for data types (blue))


Main thing 2:
Rework how variables and function calls are recognised.
* don't distinguish callsites of functions
  (don't neglect functions in rhs of assignments)
* distinguish function calls from variables in an intermediate "call" state
  (stop looking for function names in variables)
* test what comes after the $ sign in an intermediate "dollar" state
  (simplified many other states)
* recognise unparenthesized variables
  e.g. var$iable means var$(i)able
* recognise computed variable names
  e.g. $(var$(i)able)


Random fixes:
* mark illegal characters in variable names (whitespace, =, :, #)
* add the "override" keyword
* make the "filter-out" function work (the only with a dash in its name)
* color assignment operators (=, +=, :=) like operators, not as strings
* recognise singlequoted strings in function parameters
* recognise doublequoted strings in commands
* color commas in function calls
* color the whole command if the silent operator @ is prepended to it


Silly refactoring:
* rename state "Command" to "silent"
  (this is the effect I see from the @ operator)
* lowercase context names
  (I like to distinguish them from attributes)


Discussion:

Fix nesting:
The old behavior looked like failure to nest $(),
but was actually mistreatment of the function call as a variable.
The old code made an effort to distinguish the callsite of functions,
and specifically disabled recognition of function calls in the most
useful place, namely in the right hand side of assignments.
I have merged the states that previously distinguished callsites.

Color the whole command if the silent operator @ is prepended to it:
This doesn't work with line continuations due to bug 300009.
The old code only marked the first word, so arguably, this is marginally better.

Color commas in function calls:
Excessive commas behave as strings, which I have not accounted for.
(Commas are what separates function parameters,
each parameter is a space separated list of strings.)
Each function takes a fixed number of parameters, and therefore, commas.
Counting up to 2 commas, while remembering the expected number,
would require 6 states {2of2, 1of2, 0of2, 1of1, 0of1, 0of0}, times the 2
parenthesis types = 12 states vs the 2 we have now for function calls.
Comment 5 Andreas Nordal 2012-05-15 12:21:58 UTC
Created attachment 71106 [details]
old syntax file for Makefile

Since the total patch ended up containing all except 7 lines of the original, I figured I could just as well upload both versions.
Since I changed basically every line, the patch would not be much easier to apply than just taking my version as-is.
Comment 6 Christoph Cullmann 2012-10-25 19:31:11 UTC
Git commit 72da2ed2e21a078d87622ba35bd40af1aaa9311f by Christoph Cullmann.
Committed on 25/10/2012 at 21:30.
Pushed by cullmann into branch 'master'.

add improved makefile hl
thanks to Andreas Nordal

M  +151  -85   part/syntax/data/makefile.xml

http://commits.kde.org/kate/72da2ed2e21a078d87622ba35bd40af1aaa9311f
Comment 7 Dominik Haumann 2013-10-09 15:09:23 UTC
@Andreas Nordal: You meanwhile have a version 2.2 on http://kde-files.org/content/show.php?content=154532

Is it save to include this version in Kate?