script standard: tabs or spaces?

Content and general development discussion, including quest scripts and server code. TMW Classic is a project comprising the Legacy tmwAthena server & the designated improved engine server based on evolHercules.


Forum rules

This forum houses many years of development, tracing back to some of the earliest posts that exist on the board.

Its current use is for the continued development of the server and game it has always served: TMW Classic.

Post Reply

What should indentation be?

a tab
1
9%
2 spaces
0
No votes
3 spaces
0
No votes
4 spaces
10
91%
8 spaces
0
No votes
 
Total votes: 11
User avatar
o11c
Grand Knight
Grand Knight
Posts: 2262
Joined: 20 Feb 2011, 21:09
Location: ^ ^

script standard: tabs or spaces?

Post by o11c »

(for the poll, I'm going to assume that any vote for spaces would fallback to a different number of spaces before a tab)

I strongly believe that indentation should be by spaces.
In particular, I believe 4 spaces is a good idea. Among other details, it is a compromise between the fanatics who insist on 2 or 8 spaces, and it is close enough to 3 spaces. Visually, I could get used to 3 spaces, but lack of existing code counts against it (although it would be easy to convert 4 <--> 3 spaces using sed).

My reasons for preferring spaces are myriad but include:
  • Tabs are different sizes on different systems. Although the de facto standard (the VT100 terminal, which is emulated by everything these days) is 8 spaces, to others it is 4 spaces.
  • 8 columns is too much indentation. AFAIK, the most levels of indentation we have in the scripts is 3, which would be 24 columns
  • Tabs cannot be copied out of a terminal. (the only known terminal that preserves tabs is the java Terminator)
  • Tabs are defined by tabstop positions, so change size depending on where they are in the line:
    • In a diff, tabs suddenly shrink due to the insertion of a character at the beginning of a line
    • tabs are often messed up in grep's output. If you use the "start at tab stop" option, it eats
    • commented out code does not shift right. Also, a common binding is to insert a "// ", and tab after space should be avoided at all costs
  • It is difficult to count characters in a line containing tabs. This comes up more often than you might think.
  • Even if tabs are used for indentation, they should not be used for alignment. And there is no editor that handles this properly.
I have considered the following arguments for tabs
  • "tabs are mandatory syntax within the first line of an script/warp/monster/shop" - that's no reason to use them for indentation as well. Most diffs aren't within the first 3 lines of an NPC anyway
  • "it saves space" - that's not worth considering
  • "you have to press space/backspace 4 times if you use spaces for indentation" - just set your editor to use the tab key to indent, set indentation to 4 spaces, and shift-tab to unindent. Some editors automatically unindent when you press backspace in the leading whitespace
  • "tabs let each person view at their preferred width" - No. A tab is 8 spaces, and you can't change that.
  • "tabs are the existing standard" - as the data below shows, tabs do have a lead but not enough to call "standard". Also, it is much easier to convert our codebase from tab to spaces than vice versa. Simply unindent 3 times and indent once, outdent labels (easily found by regex "^ [SL]_"), then go through the few cases where you had more than 16 spaces worth of indentation. (Note that, as per the other thread, we need to review every file completely anyway.)
-----

Using the following script, I generated statistics of the existing scripts:

Code: Select all

#!/bin/bash
# Run this script from tmwa-server-{data,test}/world/map/

FILES=$(find npc/ -name '*.txt' -not -name '_*' -not -name 'monsters.txt' -not -name 'mapflags.txt' -not -name 'pvpflag.txt')

rm -f scripts-t{0,1,2}-s{0,1,2}

for FILE in $FILES
do
    TAB=0
    SP=0
    if grep -q $'^\t[[:space:]]* ' $FILE
    then
        TAB=2
    elif grep -q $'^\t' $FILE
    then
        TAB=1
    fi
    if grep -q $'^ [[:space:]]*\t' $FILE
    then
        SP=2
    elif grep -q '^ ' $FILE
    then
        SP=1
    fi

    echo $FILE >> scripts-t${TAB}-s${SP}
done

echo -n 'mixed: '; grep $'^\t[[:space:]]* ' $FILES | wc -l
echo -n 'tab:   '; grep $'^\t\+[^\t ]'      $FILES | wc -l
echo -n 'bad:   '; grep $'^ [[:space:]]*\t' $FILES | wc -l
echo -n 'space: '; grep '^ \+[^\t ]'        $FILES | wc -l
(note: the line counts are too low in the case of lines containing only space or lines containing only tabs)

Summary:

Code: Select all

305 files total
12 files (3.9%) with no indentation at all (shops and commented-out scripts)
33 files (10.8%) with only space indentation
123 files (40.2%) with only tab indentation
87 files (28.4%) with some tab indentation and some space indentation
36 files (11.8%) with some tab indentation and some bad indentation
4 files (1.3%) with some mixed indentation and no space indentation
4 files (1.3%) with some mixed indentation and some space indentation
6 files (2.0%) with some mixed indentation and some bad indentation
And, the line counts:

Code: Select all

total: 51546
mixed: 68 (0.13%)
tab:   21881 (42.4%)
bad:   160 (0.31%)
space: 14340 (27.8%)
(the rest start with nonwhitespace or are empty)
Where:
mixed indentation is an initial tab followed by spaces (I used to use this (first indentation is 4 spaces, 2nd is a tab, 3rd is a tab and 4 spaces,...), mainly so that I could still type tabs in Makefiles (before I discovered the ability to set indentation type per-filetype), but it becomes very problematic when code needs to be passed around, so I am not making it a poll option) Note that since our scripts end in .txt and the content format is not widely known it is not feasible to set a per-filetype
bad indentation is initial spaces followed by a tab (there is no reason at all to use this)

Using commands of the form:

Code: Select all

grep '^ \{2\}[^ ]' $FILES | wc -l
14363 lines starting with any number of spaces
23 of which contain only spaces, so
14340 lines using spaces as initial indentation
3 lines starting with 1 space
461 lines starting with 2 spaces
9 lines starting with 3 spaces
5098 lines starting with 4 spaces
42 lines starting with 5 spaces
4 lines starting with 6 spaces (probably 2nd level of a 3-space indentation, possible 3rd level of 2-space indentation)
7 lines starting with 7 spaces (probably tabs in a diff that got copied out of a terminal)
7904 lines starting with 8 spaces (many, but not all are 2nd level of 4-space indentation)

21 lines starting with 9 spaces
3 lines starting with 10 spaces
20 lines starting with 11 spaces
193 lines starting with 12 spaces
197 lines starting with 13 spaces (aligned with an 8 space indentation + "menu ")
2 lines starting with 14 spaces
0 lines starting with 15 spaces
317 lines starting with 16 spaces

20 lines starting with 17 spaces
7 lines starting with 18 spaces
0 lines starting with 19 spaces
0 lines starting with 20 spaces
1 line starting with 21 spaces
3 lines starting with 22 spaces
0 lines starting with 23 spaces
9 lines starting with 24 spaces

3 lines starting with 25 spaces
8 lines starting with 26 spaces
3 lines starting with 29 spaces
4 lines starting with 31 spaces
1 line starting with 45 spaces

Many of the larger, irregular indentations are attempts to align the wrapped portion of long lines (not always successful).
Former programmer for the TMWA server.
User avatar
argul
Novice
Novice
Posts: 237
Joined: 08 Aug 2010, 18:43

Re: script standard: tabs or spaces?

Post by argul »

I voted for 4 spaces.
(I would prefer any kind of spaces over tabs)
"you have to press space/backspace 4 times if you use spaces for indentation" - just set your editor to use the tab key to indent, set indentation to 4 spaces, and shift-tab to unindent. Some editors automatically unindent when you press backspace in the leading whitespace
You need to be very careful with this editor setting!, due to:
"tabs are mandatory syntax within the first line of an script/warp/monster/shop" - that's no reason to use them for indentation as well. Most diffs aren't within the first 3 lines of an NPC anyway
If you setup your editor the wrong way exactly there we get spcaes as well -> npc is commented out/not recognized.

Can we put other escape characters than tabs there?
The Header line could either use quotation marks " ' or some unusal stuff like pipes | or everything, but no tabs then.
---
User avatar
o11c
Grand Knight
Grand Knight
Posts: 2262
Joined: 20 Feb 2011, 21:09
Location: ^ ^

Re: script standard: tabs or spaces?

Post by o11c »

There's a stupid 2 stage thing ...
This might do it:

Code: Select all

diff --git a/src/map/npc.c b/src/map/npc.c
index 0335e9c..6f09777 100644
--- a/src/map/npc.c
+++ b/src/map/npc.c
@@ -2343,9 +2343,9 @@ int do_init_npc (void)
                          || (j && line[j - 1] == ',')))
                         line[j++] = ' ';
                 }
-                else if (line[i] == '\t')
+                else if (line[i] == '\t' || line[i] == '|')
                 {
-                    if (!(j && line[j - 1] == '\t'))
+                    if (!(j && (line[j - 1] == '\t' || line[j - 1] == '|')))
                         line[j++] = '\t';
                 }
                 else
Edit: I have pushed this
Former programmer for the TMWA server.
User avatar
Jenalya
TMW Adviser
TMW Adviser
Posts: 717
Joined: 22 Sep 2010, 19:28

Re: script standard: tabs or spaces?

Post by Jenalya »

So the first line of an NPC can now e.g. look like this

Code: Select all

001-1.gat,38,69,0|script|Soul Menhir|144,{
Though I like your suggestion to use whitespaces instead of tabs and the change, I think it's better if major changes are agreed by more developers before going on.
Since many people are on vacation, have exam time or something else, things are progressing only slowly and some patience might be apposite.
alastrim
Novice
Novice
Posts: 139
Joined: 02 Jun 2009, 12:19
Location: Brasil

Re: script standard: tabs or spaces?

Post by alastrim »

Just to say that I agree with the changes. ;)

Also, what about the xml files? I think we have a lot of different indentation stiles applied there too.
User avatar
o11c
Grand Knight
Grand Knight
Posts: 2262
Joined: 20 Feb 2011, 21:09
Location: ^ ^

Re: script standard: tabs or spaces?

Post by o11c »

Jenalya wrote:So the first line of an NPC can now e.g. look like this

Code: Select all

001-1.gat,38,69,0|script|Soul Menhir|144,{
Though I like your suggestion to use whitespaces instead of tabs and the change, I think it's better if major changes are agreed by more developers before going on.
Since many people are on vacation, have exam time or something else, things are progressing only slowly and some patience might be apposite.
/me learns a new word

Well, now we have official procedures, but ... just because I pushed to the server sources doesn't mean it has to be changed in the server-data (although honestly I expect it will).

I did confirm that no existing scripts used the pipe. Other characters under consideration, such as underscore, are currently used. Using some form of quotation marks is not feasible with the current scripting parser. (also single quotes are used in 1 warp and 3 scripts)

I like the pipe character because it doesn't have any horizontal stretch, so is a good visual "separator".

Note that multiple separators can (tabs and pipes mixed) be thrown together, i.e. for alignment. But I don't see how that would actually be useful:
  • scripts are long enough that indentation for adjacent scripts doesn't matter
  • mapflags are always aligned in the first 2 terms
  • mobs and warps are autogenerated. Granted, people might still want to read them, but ... existing tabs don't work too well because they are quite frequently off by a whole tab stop.
  • shops are separated by several lines of comments
alastrim wrote:Also, what about the xml files? I think we have a lot of different indentation stiles applied there too.
Client data isn't really my domain, but it seems good to me to run:

Code: Select all

find -name '*.xml' -exec xmlindent -w '{}' ';' -exec sed 's/ \+$//' -i '{}' ';'
Note that many previously "end of line" comments need to be moved to before the relevant tag.

xmlindent indents with 4 spaces by default. It appears to preserve some input formatting:
  1. if a the key=values in a <tag> was previously broken onto multiple lines, it stays.
  2. if there is a comment immediately after an opening tag, it stays on the same line.
For both, see graphics/particles/arrow-hail.xml

I am using
xmlindent -v wrote: XML Indent, version 0.2.17
Copyright (C) 2002-2004 Pekka Enberg
Former programmer for the TMWA server.
Post Reply