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

Upstream merge #1

Open
Ambrevar opened this issue Jan 19, 2018 · 8 comments
Open

Upstream merge #1

Ambrevar opened this issue Jan 19, 2018 · 8 comments

Comments

@Ambrevar
Copy link

I had a quick glance at the scripts: I could see a lot of interesting stuff in there.

I would love to integrate some of it upstream (with your permission of course :) ).

Which ones do you think would fit upstream? That is: not too specific, useful enough in the general case. I know, it's a fairly subjective question, but we can discuss this here... :)

@fictionic
Copy link
Owner

Thanks!

Well obviously the big ones are the tag- scripts. tag-contents in particular is the script that I've spent the most time on, as it's the what I'm most anal about in my music files (lol). Though it does add considerably to the runtime, so I'd want to hear any suggestions to how it could be made more efficient.

Also perhaps my division of path into basename, directory, and path? I find it helpful to be able to disable functionality on that more granular level.

@Ambrevar
Copy link
Author

Regarding basename and directory, can you provide a use-case? I'm not sure to see the point.

@fictionic
Copy link
Owner

Oh I use those all the time. They are useful when using Demlo as a general-purpose file tagger/encoder, not as a library organizer. For directory: say you want to convert a folder of flacs to mp3s, but want to make sure the mp3s end up in a separate folder. Just do mkdir mp3 then demlo -pre 'dir = "mp3" -s 01-mp3v0 -p . For basename: say you want to convert a single file from flac to mp3, and differentiate it from the other files in the directory. Just do demlo -pre 'name = "whatever" -s 01-mp3v0 -p the_file.flac.

@Ambrevar
Copy link
Author

Nice! I think this separation is indeed a better approach.

@fictionic
Copy link
Owner

Oh, another thing I think would be nice to push upstream is my replacement for the debug function, which I have called print. Basically it just supports printing nil and tables (debug currently prints an empty string for both). The table printing could be improved (if the table has a length then it only prints the numerically indexed elements up to the length, not checking for other elements), but it's pretty functional. What do you think?

@Ambrevar
Copy link
Author

I would stick to a "debug"-prefixed naming convention because those functions
won't work without the -debug flag, so the meaning is clear in my opinion.

It's misleading to call "print" and not display anything unless the -debug flag is on.

I think it's alright to include some helper functions, they are quite straightforward anyways.
That said, we should keep all the "primitives" as they are those which enable the user with the most extensibility capabilities.
Then I'd expose two functions: debug() and debugfmt(), the latter doing some formatting over tables as you suggested.

@fictionic
Copy link
Owner

Oh I have no problem with keeping it named debug(); I only renamed it to differentiate it from the builtin. In hindsight I shouldn't have.

Why do you think debug() shouldn't do formatting over tables? I was pretty confused before I figured out that it printed an empty string for nil values and tables. It could at least print table: 0x1e838a0 like the Lua REPL does.

@Ambrevar
Copy link
Author

Ambrevar commented Feb 4, 2018

That could be a bug. I'll investigate.

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