Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Fix spaces-to-tabs and tabs-to-spaces to use "tab stops" correctly #67

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix spaces-to-tabs and tabs-to-spaces to use "tab stops" correctly #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2affa5c
Fixing specs to set the config correctly between tests
giseburt May 1, 2015
2d0fb29
Fixed tabs → spaces…
giseburt May 2, 2015
67b73d5
Now spaces → tabs is working
giseburt May 2, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions lib/whitespace.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,42 @@ class Whitespace

convertTabsToSpaces: (editor) ->
buffer = editor.getBuffer()
spacesText = new Array(editor.getTabLength() + 1).join(' ')

tabLength = editor.getTabLength()

buffer.transact ->
buffer.scan /\t/g, ({replace}) -> replace(spacesText)
buffer.scan /^.*\t.*$/g, ({matchText, replace}) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like you could continue to scan for single \t characters and use the associated matche's range to determine how many spaces to replace this with rather than matching again inside the scan iterator.

while match = /^([^\t]*)\t(.*)$/.exec(matchText)
newTabLength = tabLength-(match[1].length %% tabLength)
matchText = match[1] + (new Array(newTabLength+1).join(" ")) + match[2]

replace(matchText)

convertSpacesToTabs: (editor) ->
buffer = editor.getBuffer()
spacesText = new Array(editor.getTabLength() + 1).join(' ')

tabLength = editor.getTabLength()

buffer.transact ->
buffer.scan new RegExp(spacesText, 'g'), ({replace}) -> replace('\t')
buffer.scan /^.*[ ].*$/g, ({match, replace}) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar question here... why not search for chunks of contiguous leading whitespace and use the range to break them up in a tab-stop aware way? Then you don't have to process a bunch of unrelated characters. I don't think we should be inserting tabs for anything but leading whitespace.

matchText = match[0]
spaceCount = 0
charCount = 0
outText = ""
for c in matchText
charCount++
if c is " "
spaceCount++
if charCount %% tabLength is 0
spaceCount = 0
outText += "\t"
else
if spaceCount > 0
outText += (new Array(spaceCount+1).join(" "))
spaceCount = 0
outText += c

if spaceCount > 0
outText += (new Array(spaceCount+1).join(" "))

replace(outText)
101 changes: 91 additions & 10 deletions spec/whitespace-spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ describe "Whitespace", ->
waitsForPromise ->
atom.packages.activatePackage('whitespace')

atom.config.set("whitespace.ensureSingleTrailingNewline", true)

describe "sanity checks", ->
it 'should start with the package active', ->
expect(atom.packages.isPackageActive('whitespace')).toBe true

describe "when the editor is destroyed", ->
beforeEach ->
editor.destroy()
Expand Down Expand Up @@ -59,6 +65,9 @@ describe "Whitespace", ->
beforeEach ->
atom.config.set("whitespace.removeTrailingWhitespace", false)

it "actually set whitespace.removeTrailingWhitespace to false", ->
expect(atom.config.get("whitespace.removeTrailingWhitespace")).toBe false

it "does not trim trailing whitespace", ->
editor.insertText "don't trim me \n\n"
editor.save()
Expand All @@ -77,6 +86,13 @@ describe "Whitespace", ->
describe "when 'whitespace.ignoreWhitespaceOnCurrentLine' is true", ->
beforeEach ->
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", true)
atom.config.set("whitespace.removeTrailingWhitespace", true)

it "actually set whitespace.removeTrailingWhitespace to true", ->
expect(atom.config.get("whitespace.removeTrailingWhitespace")).toBe true

it "actually set whitespace.ignoreWhitespaceOnCurrentLine to true", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnCurrentLine")).toBe true

it "removes the whitespace from all lines, excluding the current lines", ->
editor.insertText "1 \n2 \n3 \n"
Expand All @@ -88,6 +104,10 @@ describe "Whitespace", ->
describe "when 'whitespace.ignoreWhitespaceOnCurrentLine' is false", ->
beforeEach ->
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.removeTrailingWhitespace", true)

it "actually set whitespace.ignoreWhitespaceOnCurrentLine to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnCurrentLine")).toBe false

it "removes the whitespace from all lines, including the current lines", ->
editor.insertText "1 \n2 \n3 \n"
Expand All @@ -99,6 +119,17 @@ describe "Whitespace", ->
describe "when 'whitespace.ignoreWhitespaceOnlyLines' is false", ->
beforeEach ->
atom.config.set("whitespace.ignoreWhitespaceOnlyLines", false)
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.removeTrailingWhitespace", true)

it "actually set whitespace.ignoreWhitespaceOnlyLines to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnlyLines")).toBe false

it "actually set whitespace.ignoreWhitespaceOnCurrentLine to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnCurrentLine")).toBe false

it "actually set whitespace.removeTrailingWhitespace to true", ->
expect(atom.config.get("whitespace.removeTrailingWhitespace")).toBe true

it "removes the whitespace from all lines, including the whitespace-only lines", ->
editor.insertText "1 \n2\t \n\t \n3\n"
Expand All @@ -111,6 +142,17 @@ describe "Whitespace", ->
describe "when 'whitespace.ignoreWhitespaceOnlyLines' is true", ->
beforeEach ->
atom.config.set("whitespace.ignoreWhitespaceOnlyLines", true)
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.removeTrailingWhitespace", true)

it "actually set whitespace.ignoreWhitespaceOnlyLines to true", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnlyLines")).toBe true

it "actually set whitespace.ignoreWhitespaceOnCurrentLine to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnCurrentLine")).toBe false

it "actually set whitespace.removeTrailingWhitespace to true", ->
expect(atom.config.get("whitespace.removeTrailingWhitespace")).toBe true

it "removes the whitespace from all lines, excluding the whitespace-only lines", ->
editor.insertText "1 \n2\t \n\t \n3\n"
Expand Down Expand Up @@ -177,13 +219,32 @@ describe "Whitespace", ->
describe "GFM whitespace trimming", ->
beforeEach ->
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.ignoreWhitespaceOnlyLines", false)
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.removeTrailingWhitespace", true)
atom.config.set("whitespace.ensureSingleTrailingNewline", true)

waitsForPromise ->
atom.packages.activatePackage("language-gfm")

runs ->
editor.setGrammar(atom.grammars.grammarForScopeName("source.gfm"))

it "actually set whitespace.ignoreWhitespaceOnCurrentLine to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnCurrentLine")).toBe false

it "actually set whitespace.ignoreWhitespaceOnlyLines to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnlyLines")).toBe false

it "actually set whitespace.ignoreWhitespaceOnCurrentLine to false", ->
expect(atom.config.get("whitespace.ignoreWhitespaceOnCurrentLine")).toBe false

it "actually set whitespace.removeTrailingWhitespace to true", ->
expect(atom.config.get("whitespace.removeTrailingWhitespace")).toBe true

it "actually set whitespace.ensureSingleTrailingNewline to true", ->
expect(atom.config.get("whitespace.ensureSingleTrailingNewline")).toBe true

it "trims GFM text with a single space", ->
editor.insertText "foo \nline break!"
editor.save()
Expand Down Expand Up @@ -217,6 +278,7 @@ describe "Whitespace", ->
expect(editor.getText()).toBe "foo \nline break!\n"

it "respects 'whitespace.ignoreWhitespaceOnlyLines' setting", ->
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.ignoreWhitespaceOnlyLines", true)

editor.insertText "\t \nline break!"
Expand All @@ -225,6 +287,10 @@ describe "Whitespace", ->

describe "when the editor is split", ->
it "does not throw exceptions when the editor is saved after the split is closed (regression)", ->
atom.config.set("whitespace.ignoreWhitespaceOnCurrentLine", false)
atom.config.set("whitespace.ignoreWhitespaceOnlyLines", false)
atom.config.set("whitespace.ensureSingleTrailingNewline", true)

atom.workspace.getActivePane().splitRight(copyActiveItem: true)
atom.workspace.getPanes()[0].destroyItems()

Expand All @@ -234,6 +300,13 @@ describe "Whitespace", ->
expect(editor.getText()).toBe 'test\n'

describe "when deactivated", ->
beforeEach ->
waitsForPromise ->
atom.packages.activatePackage("whitespace")

it 'should start with the package active', ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why you added this...

expect(atom.packages.isPackageActive('whitespace')).toBe true

it "does not remove trailing whitespace from editors opened after deactivation", ->
atom.config.set("whitespace.removeTrailingWhitespace", true)
atom.packages.deactivatePackage('whitespace')
Expand All @@ -257,32 +330,40 @@ describe "Whitespace", ->

it "removes the trailing whitespace in the active editor", ->
atom.commands.dispatch(workspaceElement, 'whitespace:remove-trailing-whitespace')
expect(buffer.getText()).toBe "foo\nbar\n\nbaz"
expect(buffer.getText().replace(/[ ]/g, '•')).toBe "foo\nbar\n\nbaz".replace(/[ ]/g, '•')

it "does not attempt to remove whitespace when the package is deactivated", ->
atom.packages.deactivatePackage 'whitespace'
expect(buffer.getText()).toBe "foo \nbar\t \n\nbaz"
expect(buffer.getText().replace(/[ ]/g, '•')).toBe "foo \nbar\t \n\nbaz".replace(/[ ]/g, '•')

describe "when the 'whitespace:convert-tabs-to-spaces' command is run", ->
it "removes all \t characters and replaces them with spaces using the configured tab length", ->
it "removes all \t characters and replaces them with appropriate spaces using the configured tab length", ->
editor.setTabLength(2)
buffer.setText('\ta\n\t\nb\t\nc\t\td')
atom.commands.dispatch(workspaceElement, 'whitespace:convert-tabs-to-spaces')
expect(buffer.getText()).toBe " a\n \nb \nc d"
expect(buffer.getText().replace(/[ ]/g, '•')).toBe " a\n \nb \nc d".replace(/[ ]/g, '•')

buffer.setText(' \ta\n \nb\t\nc \t \td')
atom.commands.dispatch(workspaceElement, 'whitespace:convert-tabs-to-spaces')
expect(buffer.getText().replace(/[ ]/g, '•')).toBe " a\n \nb \nc d".replace(/[ ]/g, '•')

editor.setTabLength(3)
buffer.setText('\ta\n\t\nb\t\nc\t\td')
atom.commands.dispatch(workspaceElement, 'whitespace:convert-tabs-to-spaces')
expect(buffer.getText()).toBe " a\n \nb \nc d"
expect(buffer.getText().replace(/[ ]/g, '•')).toBe " a\n \nb \nc d".replace(/[ ]/g, '•')

buffer.setText(' \ta\n\t\nb\t\nc \t \td')
atom.commands.dispatch(workspaceElement, 'whitespace:convert-tabs-to-spaces')
expect(buffer.getText().replace(/[ ]/g, '•')).toBe " a\n \nb \nc d".replace(/[ ]/g, '•')

describe "when the 'whitespace:convert-spaces-to-tabs' command is run", ->
it "removes all space characters and replaces them with hard tabs", ->
it "removes appropriate space characters and replaces them with hard tabs", ->
editor.setTabLength(2)
buffer.setText(" a\n \nb \nc d")
buffer.setText(" a\n \nb \nc d\n e")
atom.commands.dispatch(workspaceElement, 'whitespace:convert-spaces-to-tabs')
expect(buffer.getText()).toBe '\ta\n\t\nb\t\nc\t\td'
expect(buffer.getText().replace(/[ ]/g, '•').replace(/\t/g, '†')).toBe '\ta\n\t \nb\t \nc\t\td\n\t\t e'.replace(/[ ]/g, '•').replace(/\t/g, '†')

editor.setTabLength(3)
buffer.setText(" a\n \nb \nc d"
buffer.setText(" a\n \nb \nc d\n e")
atom.commands.dispatch(workspaceElement, 'whitespace:convert-spaces-to-tabs')
expect(buffer.getText()).toBe '\ta\n\t\nb\t\nc\t\td')
expect(buffer.getText().replace(/[ ]/g, '•').replace(/\t/g, '†')).toBe '\ta\n\t\nb\t \nc\t\t d\n\t\t e'.replace(/[ ]/g, '•').replace(/\t/g, '†')