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

Tabs silently break indentation stripping #7834

Open
roberth opened this issue Feb 14, 2023 · 6 comments · May be fixed by #9971
Open

Tabs silently break indentation stripping #7834

roberth opened this issue Feb 14, 2023 · 6 comments · May be fixed by #9971
Labels
bug language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@roberth
Copy link
Member

roberth commented Feb 14, 2023

Describe the bug

Tabs are not supported in the indentation stripping parser. This is the right behavior in principle, but when a user does insert a tab, the behavior does not match their indentation. The right solution is to warn and let the user fix the problem. Supporting tabs is wrong, because tab width is undefined and will never be defined consistently.

Adding a warning may make '' '' strings annoying for users who have to write strings with tabs at the start of lines, but their solution is already fragile, so they should move to explicit tabs ("\t") or put the string in a separate file.

Steps To Reproduce

  1. Write a file with a '' string indented with tabs or a mix of spaces and tabs.
  2. Parsed string contains tab characters.

Expected behavior

Warn when the first tab is encountered in the indentation stripping logic. Suggest alternatives, for both cases: user wants tabs stripped, or (less likely, current behavior) user wants to keep tabs in their string.

nix-env --version output

Additional context

Add any other context about the problem here.

Priorities

Add 👍 to issues you find important.

@roberth roberth added the bug label Feb 14, 2023
@Et7f3
Copy link
Contributor

Et7f3 commented Feb 18, 2023

(less likely, current behavior) user wants to keep tabs in their string.

If not at the start ?

@roberth roberth added the language The Nix expression language; parser, interpreter, primops, evaluation, etc label Mar 16, 2023
@theotheroracle
Copy link

a current workaround is to just use spaces for that part of the code

@piegamesde
Copy link
Member

Supporting tabs is wrong, because tab width is undefined and will never be defined consistently.

I don't understand this. Tabs having a configurable width is the point of using tabs, why should this be wrong?

@thecaralice
Copy link

I do not see how tabs themselves can cause any issues. What can cause issues is mixing tabs and spaces in indentation, which should definitely be prohibited, but otherwise the parser should be fine with tabs, since it does not actually depend on indentation width, but rather on amount of indentation levels.

@thecaralice thecaralice linked a pull request Feb 8, 2024 that will close this issue
@voronind-com
Copy link

I really hope we won't end up forbidding tabs.

@ntninja
Copy link

ntninja commented Mar 12, 2024

Well, my entire system configuration uses tabs and my only actual complaint is that one needs to use a workaround to manually strip indented strings when dealing with inline Python (part of actual configuration used in production):

let
	# Helper: Workaround to remove leading indentation from generated files,
	#         since otherwise the Python scripts do not work
	fixIdentWorkaround =
		prefix: string: lib.strings.concatMapStringsSep "\n" (line: lib.strings.removePrefix prefix line) (lib.strings.splitString "\n" string);
in {
	# …
	
	siteSettings = pkgs.writeText "modoboa-site-settings.py" ((fixIdentWorkaround "\t\t" ''
		# Define special mode, induced by environment variable, that avoids
		# any external file access during the instance derivation build
		import os
		MODOBOA_BUILDING_INSTANCE = os.environ.get("MODOBOA_BUILDING_INSTANCE") == "1"

		# Secret key used as a salt in many operations
		#
		# Generated on first-run, since it needs to be managed and
		# persisted by the system between Modoboa versions, so it won’t
		# necessarily be around when we generate the instance derivation.
		if not MODOBOA_BUILDING_INSTANCE:
			with open(${escapePyString secretKeyFile}, "r") as secret_key_file:
				SECRET_KEY = secret_key_file.read().rstrip("\r\n")
		else:
			# Static dummy secret key
			SECRET_KEY = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

		# …
	'';
	# …
};

Adding a warning to “make '' '' strings annoying for users who (have to) write strings with tabs at the start of lines” would indeed be highly unwelcome, yes. Of course some Nix devs seem to have made it their personal mission to banish every last tab character in the world, since it is obviously better if we all use 2 spaces of indentation. That way, figuring out where that large opened block in that longer configuration ends stays extra hard for everyone… (Oh the terror if we had variable tab size and could make the indentation larger or smaller depending on space availability and reading requirements…!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants