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

property export #13

Open
eylles opened this issue Oct 30, 2022 · 8 comments
Open

property export #13

eylles opened this issue Oct 30, 2022 · 8 comments

Comments

@eylles
Copy link
Owner

eylles commented Oct 30, 2022

while working on #6 i found some problems with the export submodule as adding new properties isn't as straightforward as it seems.

the attempted properties under util.py

    @property
    def red_dec(self):
        """Red value as decimal."""
        return "%s" % hex_to_rgb(self.hex_color)[0]

    @property
    def green_dec(self):
        """Green value as decimal."""
        return "%s" % hex_to_rgb(self.hex_color)[1]

    @property
    def blue_dec(self):
        """Blue value as decimal."""
        return "%s" % hex_to_rgb(self.hex_color)[2]

trying to use these on a template results in a pywal error:

 File "/home/ed/.local/lib/python3.10/site-packages/pywal/export.py", line 27, in template
    new_color = util.Color(colors[cname].hex_color)
AttributeError: 'str' object has no attribute 'hex_color'

this clearly shows that there is a problem in the template exporting code as it is necesary to to specify lenght of the format string "%3s" to make the colors export, the lenght of the string should not matter at all and the end result padding with spaces may not be desirable.

there clearly was a problem with that part of the code as evidenced by a2ddff3 , in order to continue the development of this fork the important functions such as the template function of the export submodule should be clear on what they do, for that reason i'm asking @AmitPr and @loiccoyle if they can chime in to help with the debugging the problem so that we can iron out these kinks.

@eylles eylles mentioned this issue Oct 30, 2022
@loiccoyle
Copy link
Contributor

I had a quick look and I think you're running into an issue where the export.template function is modifying the colors dictionary these changes compound the more templates get rendered, leading to entries in colors which are strings which collide with entries which should be instances of utils.Color. An easy way to see this is to add some print statements.

diff --git a/pywal/export.py b/pywal/export.py
index 061c151..aabc231 100644
--- a/pywal/export.py
+++ b/pywal/export.py
@@ -14,6 +14,10 @@ def template(colors, input_file, output_file=None):
        save the file elsewhere."""
     # pylint: disable-msg=too-many-locals
     template_data = util.read_file_raw(input_file)
+    print(input_file)
+    for key, value in colors.items():
+        print(repr(key), repr(value))
     for i, l in enumerate(template_data):
         for match in re.finditer(r"(?<=(?<!\{))(\{([^{}]+)\})(?=(?!\}))", l):
             # Get the color, and the functions associated with it

You'll see that as templates get rendered more and more garbage gets added into the colors dict which collide at some point leading to what you observe. This is a consequence of the jankiness with which this was implemented in pywal.

The correct solution is to redo the templating with a more robust solution.

A bandaid fix is to work on a copy of the colors dictionary,

diff --git a/pywal/export.py b/pywal/export.py
index 061c151..aabc231 100644
--- a/pywal/export.py
+++ b/pywal/export.py
@@ -14,6 +14,10 @@ def template(colors, input_file, output_file=None):
        save the file elsewhere."""
     # pylint: disable-msg=too-many-locals
     template_data = util.read_file_raw(input_file)
+    colors = colors.copy()
     for i, l in enumerate(template_data):
         for match in re.finditer(r"(?<=(?<!\{))(\{([^{}]+)\})(?=(?!\}))", l):
             # Get the color, and the functions associated with it

@eylles
Copy link
Owner Author

eylles commented Oct 30, 2022

thanks for your input, i will test and add the bandaid for the current goal release, i would appreciate to have your input and/or collaboration to create a more robust solution for a future release.

@loiccoyle
Copy link
Contributor

Sure feel free to ping me whenever.

@eylles
Copy link
Owner Author

eylles commented Oct 30, 2022

hmmm even with the change of using a copy of the colors dict the problem persists when the length of the string is not specified, oh well still will add the commit to the release in hopes this helps with any possible problem that may happen with user templates.

@loiccoyle
Copy link
Contributor

can you give me an example template which doesn't work ?

@eylles
Copy link
Owner Author

eylles commented Oct 30, 2022

i'm using this template for testing of the properties i added recently.

# Special
background='{background}'
foreground='{foreground}'
cursor='{cursor}'

# colors
 color0='{color0}'
 color1='{color1}'
 color2='{color2}'
 color3='{color3}'
 color4='{color4}'
 color5='{color5}'
 color6='{color6}'
 color7='{color7}'
 color8='{color8}'
 color9='{color9}'
color10='{color10}'
color11='{color11}'
color12='{color12}'
color13='{color13}'
color14='{color14}'
color15='{color15}'


# colors rgbspce
 color0='{color0.rgbspace}'
 color1='{color1.rgbspace}'
 color2='{color2.rgbspace}'
 color3='{color3.rgbspace}'
 color4='{color4.rgbspace}'
 color5='{color5.rgbspace}'
 color6='{color6.rgbspace}'
 color7='{color7.rgbspace}'
 color8='{color8.rgbspace}'
 color9='{color9.rgbspace}'
color10='{color10.rgbspace}'
color11='{color11.rgbspace}'
color12='{color12.rgbspace}'
color13='{color13.rgbspace}'
color14='{color14.rgbspace}'
color15='{color15.rgbspace}'


# Colors float
 color0='{color0.red} {color0.green} {color0.blue}'
 color1='{color1.red} {color1.green} {color1.blue}'
 color2='{color2.red} {color2.green} {color2.blue}'
 color3='{color3.red} {color3.green} {color3.blue}'
 color4='{color4.red} {color4.green} {color4.blue}'
 color5='{color5.red} {color5.green} {color5.blue}'
 color6='{color6.red} {color6.green} {color6.blue}'
 color7='{color7.red} {color7.green} {color7.blue}'
 color8='{color8.red} {color8.green} {color8.blue}'
 color9='{color9.red} {color9.green} {color9.blue}'
color10='{color10.red} {color10.green} {color10.blue}'
color11='{color11.red} {color11.green} {color11.blue}'
color12='{color12.red} {color12.green} {color12.blue}'
color13='{color13.red} {color13.green} {color13.blue}'
color14='{color14.red} {color14.green} {color14.blue}'
color15='{color15.red} {color15.green} {color15.blue}'

# Colors hex
 color0='{color0.red_hex} {color0.green_hex} {color0.blue_hex}'
 color1='{color1.red_hex} {color1.green_hex} {color1.blue_hex}'
 color2='{color2.red_hex} {color2.green_hex} {color2.blue_hex}'
 color3='{color3.red_hex} {color3.green_hex} {color3.blue_hex}'
 color4='{color4.red_hex} {color4.green_hex} {color4.blue_hex}'
 color5='{color5.red_hex} {color5.green_hex} {color5.blue_hex}'
 color6='{color6.red_hex} {color6.green_hex} {color6.blue_hex}'
 color7='{color7.red_hex} {color7.green_hex} {color7.blue_hex}'
 color8='{color8.red_hex} {color8.green_hex} {color8.blue_hex}'
 color9='{color9.red_hex} {color9.green_hex} {color9.blue_hex}'
color10='{color10.red_hex} {color10.green_hex} {color10.blue_hex}'
color11='{color11.red_hex} {color11.green_hex} {color11.blue_hex}'
color12='{color12.red_hex} {color12.green_hex} {color12.blue_hex}'
color13='{color13.red_hex} {color13.green_hex} {color13.blue_hex}'
color14='{color14.red_hex} {color14.green_hex} {color14.blue_hex}'
color15='{color15.red_hex} {color15.green_hex} {color15.blue_hex}'

# Colors dec
 color0='{color0.red_dec} {color0.green_dec} {color0.blue_dec}'
 color1='{color1.red_dec} {color1.green_dec} {color1.blue_dec}'
 color2='{color2.red_dec} {color2.green_dec} {color2.blue_dec}'
 color3='{color3.red_dec} {color3.green_dec} {color3.blue_dec}'
 color4='{color4.red_dec} {color4.green_dec} {color4.blue_dec}'
 color5='{color5.red_dec} {color5.green_dec} {color5.blue_dec}'
 color6='{color6.red_dec} {color6.green_dec} {color6.blue_dec}'
 color7='{color7.red_dec} {color7.green_dec} {color7.blue_dec}'
 color8='{color8.red_dec} {color8.green_dec} {color8.blue_dec}'
 color9='{color9.red_dec} {color9.green_dec} {color9.blue_dec}'
color10='{color10.red_dec} {color10.green_dec} {color10.blue_dec}'
color11='{color11.red_dec} {color11.green_dec} {color11.blue_dec}'
color12='{color12.red_dec} {color12.green_dec} {color12.blue_dec}'
color13='{color13.red_dec} {color13.green_dec} {color13.blue_dec}'
color14='{color14.red_dec} {color14.green_dec} {color14.blue_dec}'
color15='{color15.red_dec} {color15.green_dec} {color15.blue_dec}'

as for pywal i'm using the latest commit form master of this repo with the added line to copy the color dict, when i test changing the definitions of the property red_dec the error appears again but now points to line 69

Traceback (most recent call last):
  File "/home/ed/.local/bin/wal", line 8, in <module>
    sys.exit(main())
  File "/home/ed/.local/lib/python3.10/site-packages/pywal/__main__.py", line 234, in main
    parse_args(parser)
  File "/home/ed/.local/lib/python3.10/site-packages/pywal/__main__.py", line 211, in parse_args
    export.every(colors_plain)
  File "/home/ed/.local/lib/python3.10/site-packages/pywal/export.py", line 129, in every
    template(colors, file.path, join(output_dir, file.name))
  File "/home/ed/.local/lib/python3.10/site-packages/pywal/export.py", line 69, in template
    template_data = "".join(template_data).format(**colors)
TypeError: str.format() argument after ** must be a mapping, not builtin_function_or_method
    @property
    def red_dec(self):
        """Red value as decimal."""
-        return "%3s" % hex_to_rgb(self.hex_color)[0]
+        return "%s" % hex_to_rgb(self.hex_color)[0]

@loiccoyle
Copy link
Contributor

I suspect its a variant of what I described, I'll take a look shortly.

@loiccoyle
Copy link
Contributor

Thanks, I was able to reproduce the issue.

Yeah it's related to the issue above, this seems to fix it but I haven't tested it properly so please run your own tests.

Note that the copy is no longer needed as now the colors dict is not changed in the export.template method.

diff --git a/pywal/export.py b/pywal/export.py
index 061c151..5ba35a3 100644
--- a/pywal/export.py
+++ b/pywal/export.py
@@ -55,15 +55,10 @@ def template(colors, input_file, output_file=None):

             if isinstance(new_color, util.Color):
                 new_color = new_color.strip
-            # If the color was changed, replace with a unique identifier.
+            # If replace the format placeholder with the new color
             if new_color is not colors[cname]:
                 new_color = str(new_color)
-                new_color_clean = (new_color.replace('[', '_')
-                                            .replace(']', '_')
-                                            .replace('.', '_'))
-                template_data[i] = l.replace(replace_str,
-                                             "color" + new_color_clean)
-                colors["color" + new_color_clean] = new_color
+                template_data[i] = l.replace("{" + replace_str + "}", new_color)
     try:
         template_data = "".join(template_data).format(**colors)
     except (ValueError, KeyError, AttributeError) as exc:
diff --git a/pywal/util.py b/pywal/util.py
index f949355..81d3b1d 100644
--- a/pywal/util.py
+++ b/pywal/util.py
@@ -122,17 +122,17 @@ class Color:
     @property
     def red_dec(self):
         """Red value as decimal."""
-        return "%3s" % hex_to_rgb(self.hex_color)[0]
+        return "%s" % hex_to_rgb(self.hex_color)[0]

     @property
     def green_dec(self):
         """Green value as decimal."""
-        return "%3s" % hex_to_rgb(self.hex_color)[1]
+        return "%s" % hex_to_rgb(self.hex_color)[1]

     @property
     def blue_dec(self):
         """Blue value as decimal."""
-        return "%3s" % hex_to_rgb(self.hex_color)[2]
+        return "%s" % hex_to_rgb(self.hex_color)[2]

     def lighten(self, percent):
         """Lighten color by percent."""

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants