Skip to content

tokenizer: Remove unused tabs options - #4422

Merged
vstinner merged 1 commit into
python:masterfrom
vstinner:tokenizer_alterror
Nov 17, 2017
Merged

tokenizer: Remove unused tabs options#4422
vstinner merged 1 commit into
python:masterfrom
vstinner:tokenizer_alterror

Conversation

@vstinner

Copy link
Copy Markdown
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.
Comment threadParser/tokenizer.c
altcol = (altcol/tok->alttabsize + 1)
* tok->alttabsize;
col = (col / tok->tabsize + 1) * tok->tabsize;
altcol = (altcol / ALTTABSIZE + 1) * ALTTABSIZE;

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
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:masterNov 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 freeto 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

@vstinner@benjaminp@serhiy-storchaka@the-knights-who-say-ni@bedevere-bot