search

imsnif / multitask

A mini-CI as a Zellij plugin
MIT License
73 stars 2 forks source link

Added config for filename #9

Closed cunialino closed 5 months ago

cunialino commented 5 months ago

Allow for users to specify the multitask filename and avoid overriding existing multitask files.

This allows to persist the content across sessions. My use case is keeping the same tasks' files as I spend a multiple days working on a single feature, and right now I would have to re-write the multitask file every day.

This is also my first ever PR in a public repo, so any suggestion would be very helpful!

leakec commented 5 months ago

Hi @cunialino, this is a great idea! Thanks for going through the work to write it up and create the PR :)

In order to make the review easier, can you replace the src/main.rs file with your main.rs file and similar for the multitask_file.rs? You can also replace dev.kdl with your dev_filename.kdl. Right now it looks like these are added as extra files in the main directory rather than in src, so all I see is that new files were added, but I can't easily see the diffs with the current files; I could go through and meld them manually to see diffs, but replacing them also has the added benefit of just being able to merge your changes and not needing to cleanup extra files afterward.

cunialino commented 5 months ago

Hi @leakec , thanks for your feedbacks! I'm sorry about the confusion, I started working directly on the repository and then copied the files in my fork (wrong place). Now you should see the diff correctly.

About the layout's kdl I created a new one as I thought it would be better to leave the default behaviour in case someone wanted to work on this project. Let me know if you'd rather have it merged in the same file anyway :smile: !

Any feedback on the actual code is very appreciated as I am just starting learning rust (coming from c/c++ and python)!

leakec commented 5 months ago

@cunialino No worries, thanks for making the changes. Diffs look awesome! Made a few minor tweaks and decided to merge in dev_filename.kdl into dev.kdl with a comment/uncomment to switch between the two.

Code looks great to me, but I'm not particularly experience in rust either. My background in is also in C++/Python.

Also putting some notes here for anyone that runs across this on an alternative logic I considered for formatting, but decided against in favor of what you already have. The alternative way to do this would be to skip formatting only when (1) the user explicitly passes in a multitask_file_name and (2) if the file already exists. Currently, we only format based on the latter condition. The current PR does this the safest way, preserving multitask files when possible. I like that, because presumably if a user is bringing up multitask in a directory they were already in, they want to re-run similar tasks (even if they forget to specify the multitask_file_name). Another reason to use what is already in the PR as opposed to this alternative approach is it is safer. We always preserve text when we can, which is nice since it is much easier to remove text from a file than try to re-create a multitask file that was wiped clean by formatting.

leakec commented 5 months ago

@cunialino Everything has been merged into main. Thanks again for the awesome idea and great contribution!

I plan to add a note on this the next time we release (which will happen when zelllij 0.40.0 is released) and will also give you a shout out. If for some reason I forget to add this to the release notes, feel free to ping me.