akosbalasko / zoottelkeeper-obsidian-plugin

Obsidian plugin of Zoottelkeeper: An automated folder-level index file generator and maintainer.
186 stars 13 forks source link

Towards AutoMOC - Added: File name cleaning, auto-tagging, new settings - Removed: Index File/Folder no longer listed #5

Closed MichaBrugger closed 3 years ago

MichaBrugger commented 3 years ago

Hey akos,

We've briefly spoken via email, so here I am πŸ˜‰ This is my first-ever pull request, so I might have overdone it a bit with the documentation in the README, but I hope you can forgive me, I just thought I'd rather write it all out.

Anyway, lets get to it:

I hope you like it and I'm looking forward to hearing from you! Best regards from Switzerland,

Micha

akosbalasko commented 3 years ago

Huhh, @MichaBrugger I'm so sorry, I totally missed your PR notification. I'll check your changes asap (probably tomorrow)!

MichaBrugger commented 3 years ago

@akosbalasko, no issue at all! To be honest, I'm not that sure anymore if the ideas I had were as good as I thought at the time haha

But anyway, I'll let you be the judge of that! Looking forward to your feedback :)

MichaBrugger commented 3 years ago

Generally: I will be working on solving the issues you mentioned over the weekend and will then make a new PR πŸ™ŒπŸΌ

Also, you mentioned a few times how the creation (or regeneration) of new index-files doesn't get triggered in certain cases (moving a complete folder into a subfolder, etc). The reason for that is, that the whole function is only triggered on "creation", "delete" and "rename", which obviously wouldn't be be triggered by the movement of a subfolder. However, so far I haven't found any possibility to add "moving" as a trigger event. Do you have any idea how we could solve this?

akosbalasko commented 3 years ago

@MichaBrugger On your question about 'move' as a trigger event: as far as I remember that is solved in Obsidian by two events, a "delete" and a "create" right after each other, that was the main reason why I had to wait a while and do the modifications in a batch (done by debounce function in line 33, it runs the function in each 1000 ms, just once even if there were more trigger events pushed on that timeframe.

Thank you very much, and good luck!

MichaBrugger commented 3 years ago

I've been looking into the whole thing with the broken chain: The reason is that while in higher level files the indexed file does not have the ".md" ending, while in the lower level file it does have a ".md" ending (even though it's invisible). That's the reason it doesn't properly link up.

Example (these two files would link): image

I'll try to solve that so it works with Clean File Path too πŸ‘πŸΌ

akosbalasko commented 3 years ago

Perfect, thank you! :)

MichaBrugger commented 3 years ago

I'm struggling to solve the chain-breaking issue without creating new bugs. I'm giving my best to solve it whenever I have time, but I might take a few more days. If I don't manage to solve it by Friday, I would suggest doing a partial implementation of everything that now works (which is basically everything except the "Clean String option"). What do you think?

akosbalasko commented 3 years ago

Hi @MichaBrugger !

Sure sure, I agree, partial improvements are welcomed as well, they are improvements, and it makes no sense for me to block them because of a parallel, but more complicated improvement.

Thank you for your efforts!

MichaBrugger commented 3 years ago

I was able, as far as I'm able to tell, to solve the challenges we discussed, so I made a new commit. I hope you'll like it πŸ™ŒπŸΌ

Regarding indentation: I did change everything to tabs. But unfortunately, I have auto-formatting enabled and GitHub often labels things as "new" even though they are only arranged more nicely. Sorry about that :(

akosbalasko commented 3 years ago

Hi @MichaBrugger ! Thank you, I'll check it tomorrow.

akosbalasko commented 3 years ago

Hi @MichaBrugger !

Thank you very much for your great effort, I tested your changes, and it looks like that we still have the following issues:

  1. renaming a complete (sub)folder does not rename the index file itself
  2. If a folder contains a subfolder with the same name, then it still breaks the chain (I think it stands for the case if I name the index file same as the folder)
Screenshot 2021-08-07 at 8 15 32
  1. the chain still breaks if I have a file with the same name in two subfolders and in the parent folder as well. (I think its because of the relative paths)
Screenshot 2021-08-07 at 8 16 40

I would suggest to split this PR to fine stuff and problematic stuff, I mean I gladly merge a PR that contains the autotagger feature and the cleanFiles as two different PRs. And the exclusion of the folder and the index file from the list. Could you please split these features and create a PR one per each? Thank you very much! The problematic is the relative folder linking and the index file naming after its parent folder, let's discuss it further on its specific PR .

Thanks a lot!

MichaBrugger commented 3 years ago

Hey Akos! First of all, thanks for testing and your feedback! I'm gladly going to split the PR (I'll do so today), but a few things I don't understand with the points you mentioned:

  1. You're right, it doesn't rename the index-file, but it creates a new index file with the new folder name (I left it like that on purpose, because else I would have to delete the other file (?) and I thought this to be dangerous territory. Also, as far as I can tell in the current version of Zoottelkeeper this would just break the chain if I do it on a lower level folder. image

  2. Here I'm really confused because it's an error I can't reproduce. When I have files/nested subfolders that are named the same it looks like this (even with Clean Paths on): image

  3. You're right here, that's a case that I didn't test for. It occurs if the there is a file with the same name in the "root" folder. And you're also right about the fact that this definitely is related to the clean path feature, so I'll go over that again and remove it from the current PR. image image

It might help if we could quickly chat at some point. But besides that, I'll give my best to solve these issues asap, thanks for taking your time for such a detailled feedback :)