Skip to content

Remove -t and -tt functionality from ironpython#1131

Merged
slozier merged 7 commits intoIronLanguages:masterfrom
dc366:remove_inconsistent_indentation
Mar 16, 2021
Merged

Remove -t and -tt functionality from ironpython#1131
slozier merged 7 commits intoIronLanguages:masterfrom
dc366:remove_inconsistent_indentation

Conversation

@dc366
Copy link
Copy Markdown
Contributor

@dc366 dc366 commented Mar 13, 2021

This is a PR for issue #982, which removes console flags for indentation inconsistency. As suggested by @BCSharp, the arguments parser ignores -t and -tt by taking no action on them.

I have not updated the test suite. I see that the relevant test is test_t() in test_stdconsole.py. Curiously, the test suite only fails on net46 after I make these changes.

Your comments are appreciated!

  • Ignore -t and -tt command line arguments
  • Remove -t and -tt from command line help
  • Remove sys.flags.tabcheck
  • Remove PythonOptions.IndentationInconsistencySeverity and related code

@dnfadmin
Copy link
Copy Markdown

dnfadmin commented Mar 13, 2021

CLA assistant check
All CLA requirements met.

Copy link
Copy Markdown
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

Unfortunately I think you removed some of the wrong code paths. I guess in the description it should have been noted that the expected behavior is what we would have with the -tt option turned on. That means we should keep the code that would run with IndentationInconsistencySeverity == Severity.Error.

Comment thread Src/IronPython/Compiler/Tokenizer.cs
@slozier
Copy link
Copy Markdown
Contributor

slozier commented Mar 13, 2021

As for test_stdconsole only failing on net46 it's because the whole suite is disabled in .NET Core.

Comment thread Src/IronPython/Compiler/Tokenizer.cs
@dc366
Copy link
Copy Markdown
Contributor Author

dc366 commented Mar 16, 2021

I moved the initialization of indentFormat to the State constructor. In testing on my end, there are a few new test cases which fail as a result of this change.

I realized I went beyond the scope of the original issue, from simply ignoring the command line flags for indentation warnings to forcing the tokenizer to check for inconsistent indentation (though this is the desired behavior in Python3). I am not sure I have the ability to debug the behavior of the new ReadNewline() function without spending a lot more time. Let me know what you think a reasonable stopping point is.

Copy link
Copy Markdown
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! This is great (and exactly what I had in mind as resolution of the issue).

Most of the test failures were expected with this change and I amended the tests to pass. It looks like there was one bug in the original ReadNewlineWithChecks which I fixed.

@slozier slozier merged commit 8b85383 into IronLanguages:master Mar 16, 2021
@slozier
Copy link
Copy Markdown
Contributor

slozier commented Mar 16, 2021

Thanks for the PR!

@dc366 dc366 deleted the remove_inconsistent_indentation branch March 16, 2021 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants