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

[Bug report] generate-scheme is not working #58

Open
windowsrefund opened this issue Sep 20, 2024 · 6 comments
Open

[Bug report] generate-scheme is not working #58

windowsrefund opened this issue Sep 20, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@windowsrefund
Copy link
Contributor

What image format(s) are supported by the generate-scheme command?

I've tried with a .jpg and .png and am always given an error: Error: Invalid image file supplied

I really don't know anything about rust so ability to troubleshoot is limited. That said, here's a trace:

> RUST_BACKTRACE=full tinty generate-scheme --system base16 - upload/Insert.jpg                                                                                                             1 ↵
Error: Invalid image file supplied

Caused by:
    No such file or directory (os error 2)

Stack backtrace:
   0: anyhow::context::<impl anyhow::Context<T,E> for core::result::Result<T,E>>::with_context
   1: tinty::main
   2: std::sys::backtrace::__rust_begin_short_backtrace
   3: std::rt::lang_start::{{closure}}
   4: std::rt::lang_start_internal
   5: main
   6: __libc_start_call_main
             at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
   7: __libc_start_main_impl
             at ./csu/../csu/libc-start.c:360:3
   8: _start

and obviously....

> file upload/Insert.jpg
upload/Insert.jpg: JPEG image data, JFIF standard 1.01, resolution (DPI), density 300x300, segment length 16, progressive, precision 8, 1443x1431, components 3
@windowsrefund windowsrefund added the bug Something isn't working label Sep 20, 2024
@JamyGolden
Copy link
Member

JamyGolden commented Sep 20, 2024

There may still be a bug, but the immediate error I see is in the command: tinty generate-scheme --system base16 - upload/Insert.jpg.

It should be: tinty generate-scheme --system base16 upload/Insert.jpg without that - before the image path. I should update the error message to give back a nicer error message and include the path it was given which should help identify the issue.

@windowsrefund
Copy link
Contributor Author

Using your suggestion, are you able to see this working?

@JamyGolden
Copy link
Member

I forgot to add the outfile or save in the example above. I just ran the following command and it worked correctly: tinty generate-scheme --save --system=base16 ~/Downloads/image.png

@windowsrefund
Copy link
Contributor Author

windowsrefund commented Oct 12, 2024

ah ha! I can confirm that does indeed output to ~/.local/share/tinted-theming/tinty/custom_themes/base16/tinty-generated.yaml. This is a great start. Let's see what we can learn using this as a starting place. The first batch of questions involve the fact we're working with a static output file. What happens if we run the same command against a different image? Will the tinty-generated.yaml file get clobbered?

Answer: yes

OK so now we understand the behavior of running the generate-scheme command in that form. Let's look at the actual usage info and see how we can (potentially) move away from the static output file in order to gain a bit more control.

Usage: tinty generate-scheme [OPTIONS] <OUTFILE|--save> <INFILE>

Arguments:
  <INFILE>   Which image file to use.
  [OUTFILE]  Output path to save the <slug>.yaml file to. Use '-' for stdout

This doesn't work as advertised:

tinty generate-scheme --system=base16 - upload/CD-Art.jpg

Maybe we can just lose the - and rely on the command outputting to STDOUT (which IMHO, it should really just do anyway). This doesn't work:

tinty generate-scheme --system=base16 upload/CD-Art.jpg
error: the following required arguments were not provided:
  <OUTFILE|--save>```

Let's try to satisfy the OUTFILE requirement. Nope:

tinty generate-scheme --system=base16 /tmp/foo.yaml upload/CD-Art.jpg
Error: Invalid image file supplied

So yea, I'd suggest removing the OUTFILE bits (as well as the --save and any mention of -). The command should really just focus on parsing the INFILE and just output to STDOUT so the end user can then use native shell redirection to save to a file of their choosing (if desired).

Thoughts?

Also, it would really be cleaner if everything other than the subcommand were structured as a parameter with corresponding name. That way, there's no mix-matching of --key=value and singleton args. Ideally, we'd be looking at something simple like what follows:

Just parse the image and dump to STDOUT
tinty generate-scheme --system bas16 --image /data/images/foo.png

Parse the image and save to the default file in the default path (~/.local/share/tinted-theming/tinty/custom-schemes//tinty-generated.yaml)
tinty genereate-scheme --system base16 --image /data/images/foo.png --save

Same as previous but this time define the name of the .yaml file to use when saving as well as the value of name: in the generated file:
tinty generate-scheme --system base16 --image /data/images/foo.png --save --name pandathecat

@JamyGolden
Copy link
Member

Yeah I'm with you, I think the number of options makes it confusing and for some reason the help and the actual command function has infile and outfile swapped.

I'll remove outfile as an option, print to stdout unless --save is provided as you have suggested.

Thank you for going through this and giving your input!

@windowsrefund
Copy link
Contributor Author

windowsrefund commented Oct 13, 2024

Thanks for this. I just tested it out and things are looking quite good. I do have a few questions and 1 suggestion:

There doesn't seem to be a way to populate the value of description: via the generate-scheme subcommand. Obviously, I know I can add it manually post-generation. Just figured I'd mention it.

I noticed some color values are not quoted:

  base00: '272727'
  base01: 3e3e3e
  base02: '555555'
  base03: 6d6d6d
  base04: '848484'
  base05: 9c9c9c
  base06: b3b3b3
  base07: cbcbcb
  base08: 604f50
  base09: 8b8983
  base0A: c6b5b3
  base0B: 4e5857
  base0C: 9fb0af
  base0D: 394a4c
  base0E: '555155'
  base0F: '555253'

Should they be? It seems to work regardless.

One suggestion I'd offer would be to append a commented version of the color onto the end of each line. That way, it becomes very easy to tweak a file when using an editor configured to render these color values.

tiny-generated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants