Fix terraform blockname regex

[dwc0011]

The regex for blockname does not allow empty provider blocks.
For example as mentioned by @lorengordon in #1353 the code snippet he provided had an empty provider block which is valid teraform. @Anteru also pointed out the issue with the first "}", at the time thought to be unrelated, however that is the reason why the snippet provided by @lorengordon was receiving the error.

The following valid terraform does not process correctly.
provider "aws" {}

I have updated the blockname regex to allow {}|{ this fixes the issue and passes all tests. I have added a test to the snippets and ran the golden updates command on it.
python -m pytest --update-goldens tests/snippets/terraform/test_types.txt
I then ran make test and all tests passed.

Admittedly, this is an oversimplified fix and I am working on stronger regex to deal with blocknames that are not allowed to be empty. But to get the ball rolling and ensure the tests are correct and heading down the right path I wanted to get this PR started.

If there is any reason to put more complex or fuller terraform tests under examples, for this I just followed the instructions under contributing and added a snippets test, but am wondering terraform is at the point of needing fuller more complex test case.

Please let me know if you want me to create a separate issue or use #1353 as the issue this PR addresses.

[birkenfeld]

Strange, I don't understand how the lexer leaves the blockname state without producing an error...

[dwc0011]

Strange, I don't understand how the lexer leaves the blockname state without producing an error...

Can you elaborate? Are you saying with the fix in the regex, or before the fix in the regex?

[birkenfeld]

For both cases, the blockname state has no "exit" condition.

I now re-read the code, and the lexer has an implicit "return to root state" if at line end and no rules in a state matched. This is not quite clean though, and I wonder if it should be removed.

In any case, please add a default('#pop') tot he state.

[dwc0011]

For both cases, the blockname state has no "exit" condition.

I now re-read the code, and the lexer has an implicit "return to root state" if at line end and no rules in a state matched. This is not quite clean though, and I wonder if it should be removed.

In any case, please add a default('#pop') tot he state.

Ok, I didn't look at whether the lexer was correct, once I noticed the regex I adjusted that and then ran the tests. I will look at it closer and add the default('#pop') and test again.

[lorengordon]

@birkenfeld Anything else needed here?

[jeanas]

It seems to me that just the default("#pop") suggested by Georg will do, as the closing bracket will be highlighted by the root state. You could even remove the (\s+)(\{) at the end of the regex altogether.

[dwc0011]

Removed (\s+)({) fom the regex

[jeanas]

Unfortunately, this PR still has merge conflicts with the master branch.

[dwc0011]

Unfortunately, this PR still has merge conflicts with the master branch.

I assumed we would deal with merge issue with master once everything was reviewed and agreed it is good.

I will address now.

[jeanas]

I assumed we would deal with merge issue with master once everything was reviewed and agreed it is good.

Well, it happens that updates to the merge target completely change the game as to the way a change should be done. Plus, for such a small change, it shouldn't be a burden to rebase.

[jeanas]

(NB: there are also conflicts in the tests, but you don't have to fix these by hand -- just run pytest.)

[jeanas]

Fails with regexlint in the CI, it complains that there are only 4 groups for 6 token types in bygroups(). I tend to agree :-)

[dwc0011]

Ahh, yeah, because we took out the others part. This simple PR is the gift that keeps on giving.

[jeanas]

I was surprised to see a fresh and unused state in the diff -- apparently the same issue has already been fixed by #2097 in the meantime?

[dwc0011]

I was surprised to see a fresh and unused state in the diff -- apparently the same issue has already been fixed by #2097 in the meantime?

I am glad theirs got merged, this one has been open since December and never was reviewed until recently, such is the life of open source projects. We will test the latest build and see if it fixes the issues we were addressing.

[lorengordon]

@jean-abou-samra Any idea when the 2.12 release will be, so we can see if #2097 does indeed fix this issue?

[jeanas]

@lorengordon You'd have to ask @Anteru. However, there is no need to wait on the release. Just do

pip install --user https://github.com/pygments/pygments/archive/master.zip

and (provided I've got the command right) that should give you an installation with current master. For your information, this is the token output I get for the example from #1353 (comment):

Token.Keyword.Reserved	'provider'
Token.Text.Whitespace	' '
Token.Name.Variable	'"aws"'
Token.Text.Whitespace	' '
Token.Punctuation	'{'
Token.Punctuation	'}'
Token.Text.Whitespace	'\n'
Token.Text.Whitespace	'\n'
Token.Keyword.Reserved	'module'
Token.Text.Whitespace	' '
Token.Name.Variable	'"test-lx-instance"'
Token.Text.Whitespace	' '
Token.Punctuation	'{'
Token.Text.Whitespace	'\n'
Token.Text.Whitespace	'  '
Token.Name.Attribute	'source'
Token.Text.Whitespace	' '
Token.Operator	'='
Token.Text.Whitespace	' '
Token.Literal.String.Double	'"git::https://github.com/plus3it/terraform-aws-watchmaker//modules/lx-instance/"'
Token.Text.Whitespace	'\n'
Token.Text.Whitespace	'\n'
Token.Text.Whitespace	'  '
Token.Name.Attribute	'Name'
Token.Text.Whitespace	'      '
Token.Operator	'='
Token.Text.Whitespace	' '
Token.Literal.String.Double	'"tf-watchmaker-lx-autoscale"'
Token.Text.Whitespace	'\n'
Token.Text.Whitespace	'  '
Token.Name.Attribute	'AmiId'
Token.Text.Whitespace	'     '
Token.Operator	'='
Token.Text.Whitespace	' '
Token.Literal.String.Double	'"__AMIID__"'
Token.Text.Whitespace	'\n'
Token.Text.Whitespace	'  '
Token.Name.Attribute	'AmiDistro'
Token.Text.Whitespace	' '
Token.Operator	'='
Token.Text.Whitespace	' '
Token.Literal.String.Double	'"__AMIDISTRO__"'
Token.Text.Whitespace	'\n'
Token.Punctuation	'}'
Token.Text.Whitespace	'\n'

[lorengordon]

Appreciate that, thank you!

[jeanas]

You're welcome. Do you confirm that the bug went away?

[lorengordon]

Can confirm it is now working, using pygments 2.12.0. Thanks!