Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: Injected code - Indentation of final whitespace-only line #280

Closed
1 task done
dcolestock opened this issue Jan 26, 2024 · 6 comments · Fixed by #340
Closed
1 task done

feature request: Injected code - Indentation of final whitespace-only line #280

dcolestock opened this issue Jan 26, 2024 · 6 comments · Fixed by #340
Labels
enhancement New feature or request P0 Highest priority, will receive attention

Comments

@dcolestock
Copy link

Did you check existing requests?

  • I have searched the existing issues

Describe the feature

I'd like to adjust how confirm.nvim handles a final whitespace-only line of injected code in order to prevent the end quotes from getting no indentation at all. Especially in python, its odd to see elements of an otherwise indented block of code having 0 indentation. Also, how much whitespace this final line has effects the indentation of the rest injected section.

Provide background

Here is an example of what formatting SQL code inside of python code looks like today if strings aren't defined at a global level (without indentation). Here is my example config:

require("conform").setup({
  formatters_by_ft = {
    python = { "injected" }, -- Injected only, no python formatting
    sql = { "sql_formatter" },
  }
})

And I format the following code:

class Example1:
    def test():
        query = """
            select
              a
            from
              table1
            """


class Example2:
    def test():
        query = """
            select
              a
            from
              table1
        """  # Different starting indentation for this closing quote, otherwise the same as Example1

And this is the results:

class Example1:
    def test():
        query = """
            select
              a
            from
              table1
""" # This now has no indentation


class Example2:
    def test():
        query = """
        select  -- Note indentation level is different than where the select above ended up, the only difference being the final line
          a
        from
          table1
""" # This now has no indentation also

The important things to notice are

  1. Depending on how much the closing """ was indented prior to formatting determines how much indentation the select ends up with
  2. The closing """ has been moved all the way to the left despite being in a code block that is all indented 8 spaces.

What I'd like to see happen instead:

  1. How much whitespace the final whitespace-only line has shouldn't effect what gets sent to the formatter. Its not the formatter here at issue. In the first example, the formatter recieves an indented select where Example2 the formatter recieves a select without indentation. This last line should be ignored for calculating how much indentation to strip from each line.
  2. The final indentation level of the closing """ should have nothing to do with what the formatter sent back (maybe that is true already). But, instead of ending up with 0 indentation, it should match indentation level of query = """ line at the start of the injected block, unless there are other things in the injected code at the 0 indentation level. Then it'd be okay to leave the final """ at 0. I say this last exception because in python all that extra whitespaces is actually part of the variable that is getting defined, so I suppose someone might not like their variable to contain a ton of extra whitespace characters in the string.

What is the significance of this feature?

nice to have

Additional details

No response

@dcolestock dcolestock added the enhancement New feature or request label Jan 26, 2024
@stevearc stevearc added the P0 Highest priority, will receive attention label Feb 21, 2024
@stevearc
Copy link
Owner

Can you give #340 a try and see if it fixes your issue?

@stevearc stevearc added the question Further information is requested label Mar 17, 2024
@dcolestock
Copy link
Author

Thank you @stevearc, but it doesn't seem to be working quite right:

If I start with this:

class Example1:
    def test():
        query = """
        select
          a
        from
          table1
        """

and format it once it becomes (an extra new line inserted right before the final line):

class Example1:
    def test():
        query = """
        select
          a
        from
          table1

        """

A second time it becomes (An extra new line is inserted after the spaces of the final line):

class Example1:
    def test():
        query = """
        select
          a
        from
          table1

........ # This line has 8 spaces
"""

And formatting a 3rd time becomes (removing the line with just spaces):

class Example1:
    def test():
        query = """
        select
          a
        from
          table1

"""

This is then stable without further changes.

I think the approach your just ignoring the last line of whitespace is probably the better and more conservative approach and less likely to cause issues for other people than my more aggressive suggestion of copying indentation whitespace from the query = line, which might not be where everyone wants that final """ to be. Once working as intended, I think just ignoring that whitespace will be good and let people set that final """ where they want without the formatter being too opinionated, though they'll still have to make sure on their own that they're being consistent with where they're placing that final """. This may be too niche of an issue to warrant its own setting in terms of how much whitespace that final line should have.

@github-actions github-actions bot removed the question Further information is requested label Mar 18, 2024
@stevearc
Copy link
Owner

Interesting...I tried using your injection and formatting that exact example and it's working perfectly for me. I'm using sql-formatter 15.3.0. Could you check that version and also set your log_level = vim.log.levels.TRACE and paste the output of a single format? For reference, mine looks like this:

20:08:44[DEBUG] Running formatters on /tmp/test.py: { "injected" }
20:08:44[INFO] Run injected on /tmp/test.py
20:08:44[TRACE] Input lines: { "class Example1:", "    def test():", '        query = """', "        select", "          a", "        from", "          table1", '        """' }
20:08:44[TRACE] Injected formatter regions { { "sql", 3, 19, 8, 8 } }
20:08:44[DEBUG] Injected format sql:3:8: { "sql_formatter" }
20:08:44[TRACE] Injected format lines { "", "        select", "          a", "        from", "          table1", "        " }
20:08:44[INFO] Run sql_formatter on /tmp/test.py.1.sql
20:08:44[TRACE] Input lines: { "", "select", "  a", "from", "  table1" }
20:08:44[DEBUG] Run command: { "sql-formatter" }
20:08:44[DEBUG] sql_formatter exited with code 0
20:08:44[TRACE] Output lines: { "select", "  a", "from", "  table1" }
20:08:44[TRACE] sql_formatter stderr: { "" }
20:08:44[TRACE] Injected sql:3:8 formatted lines { "select", "  a", "from", "  table1" }
20:08:44[TRACE] Applying formatting to /tmp/test.py
20:08:44[TRACE] Comparing lines { "class Example1:", "    def test():", '        query = """', "        select", "          a", "        from", "          table1", '        """' } and { "class Example1:", "    def test():", '        query = """', "        select", "          a", "        from", "          table1", '        """' }
20:08:44[TRACE] Diff indices {}
20:08:44[TRACE] Applying text edits: {}
20:08:44[TRACE] Done formatting /tmp/test.py

@stevearc stevearc added the question Further information is requested label Mar 18, 2024
@dcolestock
Copy link
Author

Sorry, that was on me. I forgot I wrote a custom python script parser which was the issue. When I use sql_formatter, it works just like you said!

My python script is supposed to be a pretty basic wrapper for sql-formatter that just stops the formatter from splitting up 1-line queries into multiple lines so query = "select * from a" won't become multiple lines (making invalid python code because " strings aren't allowed to span multiple lines).

import sys
from subprocess import PIPE, Popen

contents = sys.stdin.read()

p = Popen(["sql-formatter"], stdout=PIPE, stdin=PIPE, stderr=PIPE)
result = p.communicate(input=contents.encode())[0].decode()

# Keep as one line if started as one line to prevent
# messing up SQL in regular quotes (not always in block quotes)
if "\n" not in contents.removesuffix("\n"):
    result = " ".join(line.strip() for line in result.splitlines())

print(result)
23:03:01[TRACE] Injected formatter regions { { "sql", 3, 19, 8, 4 } }
23:03:01[DEBUG] Injected format sql:3:8: { "sqlcustom" }
23:03:01[TRACE] Injected format lines { "", "    SELECT", "      a", "    FROM", "      table1", "    " }
23:03:01[INFO] Run sqlcustom on /tmp/a.py.1.sql
23:03:01[TRACE] Input lines: { "", "SELECT", "  a", "FROM", "  table1" }
23:03:01[DEBUG] Run command: { "sqlparser" }
23:03:01[DEBUG] sqlcustom exited with code 0
23:03:01[TRACE] Output lines: { "SELECT", "  a", "FROM", "  table1", "" }
23:03:01[TRACE] sqlcustom stderr: { "" }
23:03:01[TRACE] Injected sql:3:8 formatted lines { "SELECT", "  a", "FROM", "  table1", "" }

Yeah, seems like my formatter is kicking back an extra new line at the end which kicks off the whole sequence of corrections.

Thanks so much for your help and the great plugin you created!

@github-actions github-actions bot removed the question Further information is requested label Mar 18, 2024
@dcolestock
Copy link
Author

I modified my parser to do:

result = p.communicate(input=contents.encode())[0].decode().removesuffix("\n")

And now it all works perfectly, even with my custom parser.

@stevearc
Copy link
Owner

Great news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P0 Highest priority, will receive attention
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants