Skip to content

Commit

Permalink
indent, unindent: default tab width = 4.
Browse files Browse the repository at this point in the history
  • Loading branch information
@StefanKarpinski
StefanKarpinski committed Jul 29, 2015
1 parent 151e0b0 commit 6c71b13
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 18 deletions.
4 changes: 2 additions & 2 deletions base/LineEdit.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1433,8 +1433,8 @@ AnyDict(
input = readuntil(ps.terminal, "\e[201~")[1:(end-6)]
input = replace(input, '\r', '\n')
if position(buffer(s)) == 0
indent = Base.indentation(input; tabwidth=4)[1]
input = Base.unindent(input[(indent+1):end], indent; tabwidth=4)
indent = Base.indentation(input)[1]
input = Base.unindent(input[(indent+1):end], indent)
end
edit_insert(s, input)
end,
Expand Down
4 changes: 2 additions & 2 deletions base/REPL.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,8 +787,8 @@ function setup_interface(repl::LineEditREPL; hascolor = repl.hascolor, extra_rep
input = readuntil(ps.terminal, "\e[201~")[1:(end-6)]
input = replace(input, '\r', '\n')
if position(LineEdit.buffer(s)) == 0
indent = Base.indentation(input; tabwidth=4)[1]
input = Base.unindent(lstrip(input), indent; tabwidth=4)
indent = Base.indentation(input)[1]
input = Base.unindent(lstrip(input), indent)
end
buf = copy(LineEdit.buffer(s))
edit_insert(buf,input)
Expand Down
12 changes: 7 additions & 5 deletions base/strings/io.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,13 @@ macro b_str(s); :($(unescape_string(s)).data); end
## multiline strings ##

"""
Calculate the width of leading blank space, and also return if string is blank
Calculate the width of leading blank space, and also return if string is blank.
Assumes a tab width of 4, which is standard for Julia code.
### Returns:
* (width of leading whitespace, flag if string is totally blank)
"""
function indentation(str::AbstractString; tabwidth=8)
function indentation(str::AbstractString; tabwidth=4)
count = 0
for ch in str
if ch == ' '
Expand All @@ -216,12 +217,13 @@ function indentation(str::AbstractString; tabwidth=8)
end

"""
Removes leading indentation from string
Removes leading indentation from string.
Assumes a tab width of 4, which is standard for Julia code.
### Returns:
* `ASCIIString` or `UTF8String` of multiline string, with leading indentation of `indent` removed
* `ASCIIString` or `UTF8String` of multiline string, with leading indentation of `indent` removed.
"""
function unindent(str::AbstractString, indent::Int; tabwidth=8)
function unindent(str::AbstractString, indent::Int; tabwidth=4)
indent == 0 && return str
pos = start(str)
endpos = endof(str)
Expand Down
30 changes: 21 additions & 9 deletions test/strings/io.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,16 +237,28 @@ print_joined(myio, "", "", 1)
# 11659
# The indentation code was not correctly counting tab stops
@test Base.indentation(" \t") == (8, true)
@test Base.indentation(" \tfoob") == (8, false)
@test Base.indentation(" \t \t") == (16, true)
@test Base.indentation(" \tfoob") == (4, false)
@test Base.indentation(" \t \t") == (8, true)

@test Base.unindent("\tfoo",0) == "\tfoo"
@test Base.unindent("\tfoo",4) == " foo"
@test Base.unindent("\tfoo",2) == " foo"
@test Base.unindent("\tfoo",4) == "foo"
@test Base.unindent(" \tfoo",4) == " foo"
@test Base.unindent("\t\n \tfoo",4) == " \n foo"
@test Base.unindent("\tfoo\tbar",4) == " foo bar"
@test Base.unindent("\n\tfoo",4) == "\n foo"
@test Base.unindent("\t\n \tfoo",2) == " \n foo"
@test Base.unindent("\t\n \tfoo",4) == "\n foo"
@test Base.unindent("\t\n \tfoo",6) == "\n foo"
@test Base.unindent("\t\n \tfoo",8) == "\nfoo"
@test Base.unindent("\tfoo\tbar",2) == " foo bar"
@test Base.unindent("\tfoo\tbar",4) == "foo bar"
@test Base.unindent("\n\tfoo",2) == "\n foo"
@test Base.unindent("\n\tfoo",4) == "\nfoo"
@test Base.unindent("\n \tfoo",2) == "\n foo"
@test Base.unindent("\n \tfoo",4) == "\n foo"
@test Base.unindent("\n\t\n \tfoo",4) == "\n \n foo"
@test Base.unindent("\n\tfoo\tbar",4) == "\n foo bar"

@test Base.unindent("\n \tfoo",6) == "\n foo"
@test Base.unindent("\n \tfoo",8) == "\nfoo"
@test Base.unindent("\n\t\n \tfoo",2) == "\n \n foo"
@test Base.unindent("\n\t\n \tfoo",4) == "\n\n foo"
@test Base.unindent("\n\t\n \tfoo",6) == "\n\n foo"
@test Base.unindent("\n\t\n \tfoo",8) == "\n\nfoo"
@test Base.unindent("\n\tfoo\tbar",2) == "\n foo bar"
@test Base.unindent("\n\tfoo\tbar",4) == "\nfoo bar"

7 comments on commit 6c71b13

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

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

I don't really agree with this. There is a difference between tab width, which only relates to the '\t' character, and indent amount, which is how much you indent code blocks. They don't have to be equal. The standard tab width is in fact 8. Changing the tab width to change indent amount is incorrect. I set my editor to tab width of 8 and indent amount of 4.

@StefanKarpinski
Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to revert it in that case. I'm not even sure why we have these functions in base at this point.

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

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

As usual, getting rid of them would be even better.

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

The repl code can probably get away with something much simpler than these functions for the purposes of bracketed paste.

@ScottPJones
Copy link
Contributor

Choose a reason for hiding this comment

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

Pasting code that uses standard physical tab width of 8, indent of 4 in 0.3.10:
image
You can see there are a few glitches (which is why I started investigating the bugs)

@ScottPJones
Copy link
Contributor

Choose a reason for hiding this comment

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

Pasting code into 0.4, with #11719, but not this commit:
image

Pasting code into 0.4 master, with this commit:
image

Since #12369 removed the global for the default value, there is no way now to get it working correctly again.

@StefanKarpinski
Copy link
Member Author

Choose a reason for hiding this comment

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

Having a global to configure a generic utility function seems wrong to me –  that leads to the same program inexplicably working on one system and not on another, and eventually finding that they've set their default tab size (or whatever global config) to something different. Global options like that are pretty much only ok for user interface stuff, so maybe the REPL code specifically can have a global variable instead of having a global for the indentation/unindent functions.

Please sign in to comment.