From 4acbc3d3065158516166ab20252e0ce2c25ae318 Mon Sep 17 00:00:00 2001
From: Ralf Ertzinger <ralf.ertzinger@booking.com>
Date: Thu, 16 Nov 2023 12:49:43 +0100
Subject: [PATCH] Handle %s strings with length and alignment

%s format strings allow for padding and alignment, but their behaviour
is very different from fstrings.

%20s will pad a string to 20 characters, and right align
%-20s will pad a string to 20 characters, and left align

This behaviour is carried over from the C *printf() functions.

This patch adds the ability to properly convert these to fstrings, using
the correct alignment markers.

The feature is gated behind aggressive mode for now.

The earlier code did already convert %20s, but changed the alignment
(the resulting fstring would be left aligned instead of right), and did
not understand %-20s at all.
---
 src/flynt/transform/percent_transformer.py | 28 +++++++++++++++++++++-
 test/test_edits.py                         | 18 ++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/flynt/transform/percent_transformer.py b/src/flynt/transform/percent_transformer.py
index 7dfe62e..d296091 100644
--- a/src/flynt/transform/percent_transformer.py
+++ b/src/flynt/transform/percent_transformer.py
@@ -11,7 +11,7 @@
 FORMAT_GROUP = f"[hlL]?[{FORMATS}]"
 FORMAT_GROUP_MATCH = f"[hlL]?([{FORMATS}])"
 
-PREFIX_GROUP = "[0-9]*[.]?[0-9]*"
+PREFIX_GROUP = "[+-]?[0-9]*[.]?[0-9]*"
 
 ANY_DICT = re.compile(r"(?<!%)%\([^)]+?\)")
 DICT_PATTERN = re.compile(rf"(%\([^)]+\){PREFIX_GROUP}{FORMAT_GROUP})")
@@ -53,6 +53,32 @@ def formatted_value(
         fmt_prefix = fmt_prefix.replace(".", "0")
 
     if fmt_spec in conversion_methods:
+        if fmt_spec == "s" and fmt_prefix:
+            # Strings are right aligned in percent fmt by default, and indicate
+            # left alignment through a negative prefix.
+            #
+            # fstrings left align by default, and separate signs from alignment
+            #
+            # Python even accepts float values here, for both percent fmt
+            # and fstrings
+            #
+            # In order to not have to figure out what sort of number we are
+            # dealing with, just look at the leading - sign (if there is one)
+            # and remove it for the conversion
+            if aggressive and fmt_prefix.startswith("-"):
+                # Left alignment
+                return ast_formatted_value(
+                    val,
+                    fmt_str=f"{fmt_prefix[1:]}",
+                    conversion=conversion_methods[fmt_spec],
+                )
+            if aggressive and not fmt_prefix.startswith("-"):
+                # Right alignment
+                return ast_formatted_value(
+                    val,
+                    fmt_str=f">{fmt_prefix}",
+                    conversion=conversion_methods[fmt_spec],
+                )
         if not aggressive and fmt_prefix:
             raise ConversionRefused(
                 "Default text alignment has changed between percent fmt and fstrings. "
diff --git a/test/test_edits.py b/test/test_edits.py
index cefda7d..cd58019 100644
--- a/test/test_edits.py
+++ b/test/test_edits.py
@@ -40,6 +40,24 @@ def test_call(state: State):
 
 def test_string_specific_len(state: State):
     s_in = """'%5s' % CLASS_NAMES[labels[j]]"""
+    s_expected = """f'{CLASS_NAMES[labels[j]]:>5}'"""
+
+    state.aggressive = True
+    s_out, count = code_editor.fstringify_code_by_line(s_in, state)
+    assert s_out == s_expected
+
+
+def test_string_specific_len_right_aligned(state: State):
+    s_in = """'%5s' % CLASS_NAMES[labels[j]]"""
+    s_expected = """f'{CLASS_NAMES[labels[j]]:>5}'"""
+
+    state.aggressive = True
+    s_out, count = code_editor.fstringify_code_by_line(s_in, state)
+    assert s_out == s_expected
+
+
+def test_string_specific_len_left_aligned(state: State):
+    s_in = """'%-5s' % CLASS_NAMES[labels[j]]"""
     s_expected = """f'{CLASS_NAMES[labels[j]]:5}'"""
 
     state.aggressive = True