Skip to content

tokenizer: Remove unused tabs options #4422

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

Merged
merged 1 commit into from
Nov 17, 2017
Merged

tokenizer: Remove unused tabs options #4422

merged 1 commit into from
Nov 17, 2017

Conversation

vstinner
Copy link
Member

Remove the following fields from tok_state structure which are now
used unused:

  • altwarning: "Issue warning if alternate tabs don't match"
  • alterror: "Issue error if alternate tabs don't match"
  • alttabsize: "Alternate tab spacing"

Replace alttabsize variable with ALTTABSIZE define.

Remove the following fields from tok_state structure which are now
used unused:

* altwarning: "Issue warning if alternate tabs don't match"
* alterror: "Issue error if alternate tabs don't match"
* alttabsize: "Alternate tab spacing"

Replace alttabsize variable with ALTTABSIZE define.
altcol = (altcol/tok->alttabsize + 1)
* tok->alttabsize;
col = (col / tok->tabsize + 1) * tok->tabsize;
altcol = (altcol / ALTTABSIZE + 1) * ALTTABSIZE;
Copy link
Member

Choose a reason for hiding this comment

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

This is just altcol++, isn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know well this code, so I prefer to minimize my changes to avoid any risk of regression.

It looks like the code doesn't round properly for tabsize=1. But I was too lazy to try to understand this old code. I prefer to leave it as it is if something has to change the code later.

FYI I found this code when I wanted to enable tabs/spaces warning in my new developer mode... Then I understood that the warning is always emitted and it's even an hard error...

Copy link
Member

Choose a reason for hiding this comment

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

I recently learned a new term: Chesterton's fence 😄.

Yes, since we are not going to allow mixing spaces and tabs, altwarning and alterror can be removed. But the algorithm not always works properly (see bpo-32053), and I think that it would be better at least to write it in simpler way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please go ahead and continue to cleanup this code ;-) Be for my side, I'm done. As I wrote, I only cared when the syntax warning was emitted (and it's now always emitted).

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to not introduce useless and misleading parameters like ALTTABSIZE.

And, to be pedantic, the syntax warning is now never emitted (the syntax error is emitted instead).

@vstinner vstinner merged commit f2ddc6a into python:master Nov 17, 2017
@vstinner vstinner deleted the tokenizer_alterror branch November 17, 2017 09:25
jimmylai added a commit to jimmylai/cpython that referenced this pull request Nov 17, 2017
* 'master' of https://github.com/python/cpython: (550 commits)
  bpo-31867: Remove duplicates in default mimetypes. (python#4388)
  tokenizer: Remove unused tabs options (python#4422)
  bpo-31691: Specify where to find build instructions for the Windows installer (python#4426)
  Fix typo in atexit documentation. (pythonGH-4419)
  bpo-31702: Allow to specify rounds for SHA-2 hashing in crypt.mksalt(). (python#4110)
  bpo-32043: New "developer mode": "-X dev" option (python#4413)
  bpo-30349: Raise FutureWarning for nested sets and set operations (python#1553)
  bpo-32037: Use the INT opcode for 32-bit integers in protocol 0 pickles. (python#4407)
  bpo-30143: 2to3 now generates a code that uses abstract collection classes (python#1262)
  bpo-32030: Enhance Py_Main() (python#4412)
  bpo-32030: Split Py_Main() into subfunctions (python#4399)
  bpo-32034: Make IncompleteReadError & LimitOverrunError pickleable python#4409
  bpo-32025: Add time.thread_time() (python#4410)
  bpo-32018: Fix inspect.signature repr to follow PEP 8 (python#4408)
  bpo-30399: Get rid of trailing comma in the repr of BaseException. (python#1650)
  bpo-30950: Convert round() to Argument Clinic. (python#2740)
  bpo-32011: Revert "Issue python#15480: Remove the deprecated and unused TYPE_INT64 code from marshal." (python#4381)
  bpo-32023: Disallow genexprs without parenthesis in class definitions. (python#4400)
  bpo-31949: Fixed several issues in printing tracebacks (PyTraceBack_Print()). (python#4289)
  bpo-32032: Test both implementations of module-level pickle API. (python#4401)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants