Repository: domluna/JuliaFormatter.jl
Pull request #41 (2019-08-24 15:46:29)

Don't transform global docstrings
Previously we did a small transform on a global docstring where single quotes were replaced with triple quotes and the documentation content was wrapped with newlines such that it the result would be:

```julia
"""
doc
"""
Foo
```

I'm beginning to think this might not be worth it.

1. As pointed out in #37, the transformation in some cases results in a unwanted result. There are other cases where a one line docstring is desirable.
2. #38 pointed out during multiple iterations of a format it's possible for the docstring to produce a slightly different result. I've verified this with a sample input found in #37. 

This PR get rids of the transformation and treats the docstring how we treat any string. By doing this it looks 1) and 2) are solved. Note the formatter still aligns the docstring to the match the indentation of what it's documenting.

Additionally during tests we now verify on a double formatted output.




Patch:
diff --git a/src/pretty.jl b/src/pretty.jl
index 2ee4845c..158d2d36 100644
--- a/src/pretty.jl
+++ b/src/pretty.jl
@@ -352,7 +352,7 @@ function p_punctuation(x, s)
     PTree(x, loc[1], loc[1], val)
 end
 
-function p_literal(x, s; include_quotes = true)
+function p_literal(x, s)
     loc = cursor_loc(s)
     if !is_str_or_cmd(x.kind)
         s.offset += x.fullspan
@@ -376,13 +376,6 @@ function p_literal(x, s; include_quotes = true)
     startline, endline, str = str_info
     # @debug "" loc startline endline str
 
-    if !include_quotes
-        idx = startswith(str, "\"\"\"") ? 4 : 2
-        str = str[idx:end-idx+1]
-        str = strip(str, ' ')
-        str[1] == '\n' && (str = str[2:end])
-        str[end] == '\n' && (str = str[1:end-1])
-    end
     s.offset += x.fullspan
 
     lines = split(str, "\n")
@@ -399,7 +392,7 @@ function p_literal(x, s; include_quotes = true)
         end
     end
 
-    # @debug "" include_quotes lines x.val loc loc[2] sidx
+    # @debug "" lines x.val loc loc[2] sidx
 
     t = PTree(CSTParser.StringH, -1, -1, loc[2], 0, nothing, PTree[], Ref(x))
     for (i, l) in enumerate(lines)
@@ -412,17 +405,10 @@ function p_literal(x, s; include_quotes = true)
 end
 
 # StringH
-function p_stringh(x, s; include_quotes = true)
+function p_stringh(x, s)
     loc = cursor_loc(s)
     startline, endline, str = s.doc.lit_strings[s.offset-1]
 
-    if !include_quotes
-        idx = startswith(str, "\"\"\"") ? 4 : 2
-        str = str[idx:end-idx+1]
-        str = strip(str, ' ')
-        str[1] == '\n' && (str = str[2:end])
-        str[end] == '\n' && (str = str[1:end-1])
-    end
     s.offset += x.fullspan
 
     lines = split(str, "\n")
@@ -441,7 +427,7 @@ function p_stringh(x, s; include_quotes = true)
         end
     end
 
-    # @debug "" include_quotes lines x.val loc loc[2] sidx
+    # @debug "" lines x.val loc loc[2] sidx
 
     t = PTree(x, loc[2])
     for (i, l) in enumerate(lines)
@@ -460,45 +446,22 @@ function p_macrocall(x, s)
     if x.args[1].typ === CSTParser.GlobalRefDoc
         loc = cursor_loc(s)
         # x.args[1] is empty and fullspan is 0 so we can skip it
-        add_node!(
-            t,
-            PTree(
-                CSTParser.LITERAL,
-                loc[1],
-                loc[1],
-                nspaces(s),
-                3,
-                "\"\"\"",
-                nothing,
-                nothing
-            )
-        )
         if x.args[2].typ === CSTParser.LITERAL
-            add_node!(t, p_literal(x.args[2], s, include_quotes = false), max_padding = 0)
+            add_node!(t, p_literal(x.args[2], s), max_padding = 0)
         elseif x.args[2].typ == CSTParser.StringH
-            add_node!(t, p_stringh(x.args[2], s, include_quotes = false))
+            add_node!(t, p_stringh(x.args[2], s))
         end
         loc = cursor_loc(s)
-        add_node!(
-            t,
-            PTree(
-                CSTParser.LITERAL,
-                loc[1] - 1,
-                loc[1] - 1,
-                nspaces(s),
-                3,
-                "\"\"\"",
-                nothing,
-                nothing
-            ),
-            max_padding = 0
-        )
         add_node!(t, pretty(x.args[3], s), max_padding = 0)
         return t
     end
 
     multi_arg = length(x) > 5 && CSTParser.is_lparen(x.args[2]) ? true : false
 
+    has_closer = is_closer(x.args[end])
+
+    # @info "" has_closer
+
     # same as CSTParser.Call but whitespace sensitive
     for (i, a) in enumerate(x)
         n = pretty(a, s)
@@ -519,9 +482,16 @@ function p_macrocall(x, s)
         elseif CSTParser.is_comma(a) && i < length(x) && !is_punc(x.args[i+1])
             add_node!(t, n, join_lines = true)
             add_node!(t, Placeholder(1))
-        elseif a.fullspan - a.span > 0 && i < length(x)
-            add_node!(t, n, join_lines = true)
-            add_node!(t, Whitespace(1))
+        elseif a.fullspan - a.span > 0
+            if has_closer && i < length(x) - 1
+                add_node!(t, n, join_lines = true)
+                add_node!(t, Whitespace(1))
+            elseif !has_closer && i < length(x)
+                add_node!(t, n, join_lines = true)
+                add_node!(t, Whitespace(1))
+            else
+                add_node!(t, n, join_lines = true)
+            end
         else
             add_node!(t, n, join_lines = true)
         end


Test patch:
diff --git a/test/files/issue_38.jl b/test/files/issue_38.jl
new file mode 100644
index 00000000..2ecf3be8
--- /dev/null
+++ b/test/files/issue_38.jl
@@ -0,0 +1,31 @@
+
+
+"""
+Free space in Gb at the given path.
+"""
+free_space(path) = shutil.disk_usage(path)[3] / 1000_000_000
+
+
+settings = ArgParseSettings()
+
+@add_arg_table settings begin
+        "--file"
+        help = """
+        path to the file.
+        ...
+        """
+end
+
+@add_arg_table settings begin
+        "--output", "-o"
+        help = "output..."
+        default = "."
+        """
+        --file
+        """
+        help = """
+        path to the file.
+        ...
+        """
+        default = ""
+end
diff --git a/test/files/setup_databases.jl b/test/files/setup_databases.jl
new file mode 100644
index 00000000..8e35dfa3
--- /dev/null
+++ b/test/files/setup_databases.jl
@@ -0,0 +1,151 @@
+#!/usr/bin/env julia
+
+using ArgParse
+using BioStructures
+using DataDeps
+using PyCall
+
+const shutil = pyimport("shutil")
+
+"Free space in Gb at the given path."
+free_space(path) = shutil.disk_usage(path)[3] / 1000_000_000
+
+function check_space(free, min_space, database_name)
+    if free < min_space
+        throw(ErrorException("""
+        Free space: $free Gb
+        Please make sure to have at least $min_space Gb of free disk space
+        before downloading the $database_name database.
+        """))
+    end
+end
+
+function parse_commandline()
+    settings = ArgParseSettings(description="""
+    This script set up the needed databases for PhyloSofS in the `--output`
+    path. If the `--pdb`, `--uniclust` or `--pdb70` argument is used, the
+    script is going to create a symbolic link to the indicated folder
+    instead of downloading the database.
+
+    It creates a `databases` folder in `--output` containing three folders:
+    `pdb`, `uniclust` and `pdb70`
+    """,)
+
+    @add_arg_table settings begin
+        "--output", "-o"
+            help = "path where the database folder is going to be created."
+            default = "."
+        "--pdb"
+            help = """
+            path to an already existing local folder containing the entire pdb
+            in mmCIF format.
+            """
+            default = ""
+        "--uniclust"
+            help = """
+            path to an already existing local folder containing the uniclust
+            database from the HH-suite databases.
+            """
+            default = ""
+        "--uniclust_version"
+            help = """
+            Uniclust30 version to be downloaded: YYYY_MM
+            """
+            default = "2018_08"
+        "--pdb70"
+            help = """
+            path to an already existing local folder containing the
+            pdb70_from_mmcif database from the HH-suite databases.
+            """
+            default = ""
+    end
+
+    return parse_args(settings)
+end
+
+function main()
+    execution_folder = pwd()
+    parsed_args = parse_commandline()
+
+    output_path = joinpath(abspath(parsed_args["output"]), "databases")
+    mkpath(output_path)
+
+    pdb = parsed_args["pdb"]
+    uniclust = parsed_args["uniclust"]
+    pdb70 = parsed_args["pdb70"]
+
+    uniclust_version = parsed_args["uniclust_version"]
+
+    pdb_path = joinpath(output_path, "pdb")
+    uniclust_path = joinpath(output_path, "uniclust")
+    pdb70_path = joinpath(output_path, "pdb70")
+
+    free = free_space(output_path)
+
+    @info "Setting up Uniclust30 (HH-suite database)"
+
+    if uniclust == ""
+        check_space(free, 14, "HH-suite Uniclust30")
+        cd(output_path)
+        download(string(
+            "http://wwwuser.gwdg.de/~compbiol/uniclust/2018_08/uniclust30_",
+            uniclust_version, ".tar.gz"
+            ), "uniclust.tar.gz")
+        unpack("uniclust.tar.gz")
+        mv("uniclust30_$uniclust_version", "uniclust")
+        cd(execution_folder)
+    else
+        uniclust = abspath(uniclust)
+        mkpath(uniclust_path)
+        for file in readdir(uniclust)
+            if filesize(file) > 0
+                symlink(joinpath(uniclust, file), joinpath(uniclust_path, file))
+            end
+        end
+    end
+
+    cd(uniclust_path)
+    to_erase = string("_", uniclust_version)
+    for file in readdir()
+        if occursin(to_erase, file)
+            if filesize(file) > 0
+                symlink(file, replace(file, to_erase => ""))
+            end
+        end
+    end
+    cd(execution_folder)
+
+    @info "Setting up pdb70 (HH-suite database)"
+
+    if pdb70 == ""
+        check_space(free, 42, "HH-suite pdb70_from_mmcif")
+        mkpath(pdb70_path)
+        cd(pdb70_path)
+        download("http://wwwuser.gwdg.de/~compbiol/data/hhsuite/databases/hhsuite_dbs/pdb70_from_mmcif_latest.tar.gz",
+            "pdb70.tar.gz")
+        unpack("pdb70.tar.gz")
+        cd(execution_folder)
+    else
+        pdb70 = abspath(pdb70)
+        mkpath(pdb70_path)
+        for file in readdir(pdb70)
+            if filesize(file) > 0
+                symlink(joinpath(pdb70, file), joinpath(pdb70_path, file))
+            end
+        end
+    end
+
+    @info "Setting up PDB (mmCIF format)"
+
+    if pdb == ""
+        check_space(free, 157, "PDB")
+        mkpath(pdb_path)
+        downloadentirepdb(pdb_dir=pdb_path, file_format=MMCIF)
+    else
+        pdb = abspath(pdb)
+        symlink(pdb, pdb_path)
+    end
+
+end
+
+main()
diff --git a/test/runtests.jl b/test/runtests.jl
index 2a7959ca..81d5130c 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -3,7 +3,14 @@ import JuliaFormatter
 using CSTParser
 using Test
 
-fmt(s, i = 4, m = 80) = JuliaFormatter.format_text(s, indent = i, margin = m)
+fmt1(s, i = 4, m = 80) = JuliaFormatter.format_text(s, indent = i, margin = m)
+
+# Verifies formatting the formatted text
+# results in the same output
+function fmt(s, i = 4, m = 80)
+    s1 = fmt1(s, i, m)
+    fmt1(s1, i, m)
+end
 
 function run_pretty(text::String, print_width::Int)
     d = JuliaFormatter.Document(text)
@@ -870,58 +877,70 @@ end
         t = run_pretty(str, 80)
         @test length(t) == 12
 
-        @test fmt("""
+        str = """
         \"""doc
         \"""
         function f()
             20
-        end""") == str
+        end"""
+        @test fmt(str) == str
 
-        @test fmt("""
+        str = """
         \"""
         doc\"""
         function f()
             20
-        end""") == str
+        end"""
+        @test fmt(str) == str
 
-        @test fmt("""
+        str = """
         \"""doc\"""
         function f()
             20
-        end""") == str
+        end"""
+        @test fmt(str) == str
 
-        @test fmt("""
+        str = """
         "doc
         "
         function f()
             20
-        end""") == str
+        end"""
+        @test fmt(str) == str
 
-        @test fmt("""
+        str = """
         "
         doc"
         function f()
             20
-        end""") == str
+        end"""
+        @test fmt(str) == str
 
-        @test fmt("""
+        str = """
         "doc"
         function f()
             20
-        end""") == str
+        end"""
+        @test fmt(str) == str
 
         # test aligning to function identation
-        @test fmt("""
+        str_ = """
             "doc"
         function f()
             20
-        end""") == str
+        end"""
+        str = """
+        "doc"
+        function f()
+            20
+        end"""
+        @test fmt(str_) == str
 
         str = """\"""
                  doc for Foo
                  \"""
                  Foo"""
-        @test fmt("\"\"\"doc for Foo\"\"\"\nFoo") == str
+        @test fmt(str) == str
         t = run_pretty(str, 80)
         @test length(t) == 11
 
@@ -946,14 +965,12 @@ end
         str = """error("foo\\n\\nbar")"""
         @test fmt(str) == str
 
-        # nesting
-
         str = """
         \"""
         \\\\
         \"""
         x"""
-        @test fmt("\"\\\\\" x") == str
+        @test fmt(str) == str
 
         str = """
         begin