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

Is there a good reason to err on tabs in build files after all? #1598

Open
xparq opened this issue Jul 9, 2019 · 1 comment · May be fixed by #2392
Open

Is there a good reason to err on tabs in build files after all? #1598

xparq opened this issue Jul 9, 2019 · 1 comment · May be fixed by #2392
Labels

Comments

@xparq
Copy link

xparq commented Jul 9, 2019

Seen the closed issue #394 as "intended behavior", but even more significantly the open one (from 4 years later): #952. Quoting @Alhadis's comment from there:

You might consider lifting the ban on tabs, then, because enforcing a specific style
of formatting is just repeating Make's mistake.

Yes, please. Or at least avoid "silently" erring on tabs (by "silently" I mean confusing error messages like "a line with 'command =' is expected", for a line that apparently reads just that; it was my first obstacle (well, second, after a "linting error" for a BOM) that I stumbled across in my hello-world attempt to evaluate whether I should consider switching to Ninja (until finally switching to redo some day? :) ).

BTW, realizing that the tab might be the problem was made even less obvious by the explicit notion that Ninja is (trying to be) a close kin of make (where tabs are the rule).

And regarding @Alhadis's other point: "The lexer should raise an error only if tabs and spaces are mixed within the same file, which (of course) should be forbidden." -- considering Evan's comment of

Re 3, ... nested scopes of indenting (like Haskell "where" clauses) and
then it never ended up being useful. I agree the docs could remove all mention of
parent and simplify to just "indented or not".

then even mixing tabs and spaces should be none of Ninja's business, as far as I understand. Until e.g. nested scopes may get introduced, of course. But even then, only an indent string of mixed tabs and spaces at line-start should be a problem (not any mixing in general).

Thanks, and a nice job overall!

(Note: I'm all new to Ninja, and ended up here only due to my own tab affair, as the manual is still silent about tabs -- which is fine, as in this case possibly many of us would prefer changing the code, not the docs.)

@jhasse
Copy link
Collaborator

jhasse commented Dec 2, 2020

Silently erring on tabs or confusing error messages are separate issues, it should always say "tabs are not allowed, use spaces".

[...] as the manual is still silent about tabs [...]

The manual had a short paragraph on tabs, but it was (mistakenly) removed when \r handling was introduced for Windows. The good reason is there:

To simplify some code [...]

So if there's a way to handle tabs without over-complicating the code, PRs are welcome.

pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 13, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
@pkitszel pkitszel linked a pull request Mar 13, 2024 that will close this issue
pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 18, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 19, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

tests: print string instead of int for Lexer::Token

Extend C++ tests by a wrapper for Lexer::Token printing, to have string
value reported instead of numeric value, that gives "newline" instead of
"8" in test results.
Original line numbers are kept in error messages as they were before.

Extend python functional test to have TABs used in both variable and build
statements.

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 19, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

tests: print string instead of int for Lexer::Token

Extend C++ tests by a wrapper for Lexer::Token printing, to have string
value reported instead of numeric value, that gives "newline" instead of
"8" in test results.
Original line numbers are kept in error messages as they were before.

Extend python functional test to have TABs used in both variable and build
statements.

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 19, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

tests: print string instead of int for Lexer::Token

Extend C++ tests by a wrapper for Lexer::Token printing, to have string
value reported instead of numeric value, that gives "newline" instead of
"8" in test results.
Original line numbers are kept in error messages as they were before.

Extend python functional test to have TABs used in both variable and build
statements.

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 22, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

tests: print string instead of int for Lexer::Token

Extend C++ tests by a wrapper for Lexer::Token printing, to have string
value reported instead of numeric value, that gives "newline" instead of
"8" in test results.
Original line numbers are kept in error messages as they were before.

Extend python functional test to have TABs used in both variable and build
statements.

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
pkitszel added a commit to pkitszel/ninja that referenced this issue Mar 22, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

tests: print string instead of int for Lexer::Token

Extend C++ tests by a wrapper for Lexer::Token printing, to have string
value reported instead of numeric value, that gives "newline" instead of
"8" in test results.
Original line numbers are kept in error messages as they were before.

Extend python functional test to have TABs used in both variable and build
statements.

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
pkitszel added a commit to pkitszel/ninja that referenced this issue Apr 22, 2024
Allow TAB character to be used for indentation.

This is useful to have TAB character used as indentation, especially when
parts of build.ninja are hand-written as HEREDOCs in otherwise
TAB-indented file (either mandated by style for other part of project, or
required by language itself).

Changing lexer is easy thanks to the use of re2c, syntax is perhaps a bit
too permissive now, but that is job of the parser to reject use of mixed
indentation.

Let's stop complaining that:
ninja: error: build.ninja:3: expected 'command =' line
when it is exactly:
	command = cc $cflags -c $in -o $out

tests: print string instead of int for Lexer::Token

Extend C++ tests by a wrapper for Lexer::Token printing, to have string
value reported instead of numeric value, that gives "newline" instead of
"8" in test results.
Original line numbers are kept in error messages as they were before.

Extend python functional test to have TABs used in both variable and build
statements.

Closes ninja-build#1598
Signed-of-by: Przemek Kitszel <pkitszel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants