Replies: 5 comments 17 replies
-
|
Starting refactoring the configuration management a bit (see 683f512). Hopefully I can get this done by the end of the week-end. The configuration itself will not change, so no breaking changes or anything like that are to be expected. I basically want to move it all out of the main file and make it easier to access from other modules. Now I have to edit the billion calls to |
Beta Was this translation helpful? Give feedback.
-
|
Almost done. The refactor branch should be stable. It would be great if someone had the time to test it a bit. |
Beta Was this translation helpful? Give feedback.
-
|
I had some delays due to the holidays and needing to spend some time learning the internals of NeoVim first, but I'm getting started on the refactor. I'm starting by trying to get main merged into refact. General process for the bit I've done so far has been to take new code from main but make sure it's using the names as changed in refact when, e.g., config was pushed into a separate file. So far, I haven't seen any new functionality in refact, which makes sense since it's a refactor branch, so there hasn't been any real issue with it, but I'm not super deep in. Once I get that done, I want to go through and take a look at each function in telekasten.lua and summarize the key steps in what it's doing. With that, it should be easier to see how functions should be split, combined, and moved to new files. It will also be a good time to make sure paths are being handled by plenary.path(). Finally, after those, I should be able to handle the markdown links. Syntax highlighting seems like it's probably going to require more specialized knowledge than I currently have or am likely to be able to develop soon, so I will likely need to leave that to someone else. I'm hoping to finish merging main in the next couple days, but it may take until some time next week. Would really like to have the markdown links working ASAP, so I'm hoping I can keep chugging along at a good pace. |
Beta Was this translation helpful? Give feedback.
-
|
I'm at 50% by line count, so I suppose it's about time to start talking about how best to handle the pull requests. First off, I'm thinking I should make a request for the main merge as soon as that's done and get refact updated in the official repo. From there, probably basically one request per bullet point as you listed it. Maybe split the util function moves into one per util file, but probably only if the work ends up being substantial. I feel like it shouldn't be too rough, though. Should I bother making a separate pull request for all the prep comments I'm planning to make next? |
Beta Was this translation helpful? Give feedback.
-
Definitely. That would be a nice starting point
Yes. I would rather have to review 3 smaller PRs rather than a gigantic one. So if you feel it could be splitted, do not hesitate.
It is up to you. I have no issue with just having a look at that and maybe take the opportunity to discuss a bit implementation before you start working. |
Beta Was this translation helpful? Give feedback.
-
|
Sounds good. As for the comments pull request, I'll probably do it just in case anything happens again and I don't get to finish. I'm hoping I do, but by that point, it's a lot of effort to figure all that out, and it might as well get documented. A lot of the decisions for the rest of it will either be made there or heavily simplified and influenced by the info they have. |
Beta Was this translation helpful? Give feedback.
-
|
I've finished a first pass at the merge. I had to resolve a lot of conflicts in telekasten.lua, add the media_extensions config option to telekasten/config.lua, and update the telekasten/util/files.lua functions check_dir_and_ask and global_dir_check with changes that got incorrectly merged back into telekasten.lua. Any other changes were automatically resolved by git and are so far unchecked. I want to look over everything again with fresh eyes tomorrow (hopefully) to make sure I didn't make and miss any mistakes. If you want to preview the changes, I've pushed them to my fork, and you can see the commit with these changes here. Otherwise, I'm hoping I can get a pull request made tomorrow. Should be a very low chance of anything being newly broken. |
Beta Was this translation helpful? Give feedback.
-
|
Pull request has officially been made for merging main into refact. As noted in the request, I don't have a full vault of my own to do much testing with. I made a few basic notes and tested some basic stuff, but I'm not well equipped for extensive testing. |
Beta Was this translation helpful? Give feedback.
-
|
@renerocksai, looks like @TheTaoOfSu is getting pretty involved in Telekasten and doing significant work here. You discussed previously the possibility to move Telekasten to an organization rather than a personal repo. This may drive more contributors to the project and simplify the development. Could you try to find some time to put that in motion? I can be an admin for the organization alongside you if you are ok with that so I can manage everything and not bother you anymore 😄 |
Beta Was this translation helpful? Give feedback.
-
|
Obviously, if @TheTaoOfSu is interested, we could make them a developer in the organization so they can directly push to some feature branches etc if they are interested. |
Beta Was this translation helpful? Give feedback.
-
|
I like the idea of continuing to help, but I can't guarantee I'll be able to commit a lot afterwards. I suppose it would depend on a few things like the direction Telekasten takes, how fast things move, and how much free time I keep long term for coding. Development hasn't been particularly fast paced lately, so I could probably at least help occasionally. |
Beta Was this translation helpful? Give feedback.
-
|
It is done! Telekasten is now free! @lambtho12 , I have made you an owner of this org. Feel free to manage anything (I mean it) to your liking. I cannot express my gratitude in words; but I am very grateful! Thank you all (incl. @TheTaoOfSu) for keeping telekasten alive, for all the hard work, and improving the status quo! |
Beta Was this translation helpful? Give feedback.
-
|
Thank you for starting this awesome project! I hope we can continue improving it and make it a bit more simple to use/expand for new users. |
Beta Was this translation helpful? Give feedback.
-
|
Is there some decent documentation on Telescope and/or Plenary? Figuring things out straight from the code gets cumbersome when the functions use other internal functions that may themselves sometimes use other internal functions... It doesn't seem super important that I understand exactly how some of these functions work, but it'd be nice to. |
Beta Was this translation helpful? Give feedback.
-
|
Telescope has a wiki and a developer documentation. For plenary, I do not think there is any proper documentation. Their README just contains the basic module names and a few indications. Instead of directly looking into the implementation of each function to discover what it does, you could maybe just have a look at the unit tests to have an idea of what to expect without needing to review the entire implementation. |
Beta Was this translation helpful? Give feedback.
-
|
I just finished a first pass at commenting all the functions in telekasten.lua. Primarily, this focused on comments at the start of function definitions to explain a bit and note a potential destination file, although I did sprinkle in a few TODOs and mid-function clarifications along the way, so it's not all focused on the refactor. Some will likely be worth keeping, some will be cleared as TODOs are completed and the refactor progresses, but some I'm not sure. One such thing I included was a note of the parameters' and return values' types in the form "param type, param type,... -> return type, return type,...". Although my Lua knowledge is limited, its flexibility suggests this may be a little unusual for Lua code, but I personally find you can get a lot of basic questions about functions answered relatively quickly if you can just clearly tell the types of the inputs and outputs combined with a decent function name, even if the rest of the comments are lacking or nonexistent. I did notice that a large majority of functions seemed fit for utils/files.lua, so I'm going to step away for a bit before I reevaluate some of it and see if I've been neglecting some other choices. Either way, soon after, I'll start moving functions around and doing my best to make sure functions are still addressed appropriate wherever they may be called from. I don't think a lot of these comments need to be pushed upstream, so I won't make a request for what I have right now. If you want to check it out, you can see the new telekasten.lua here or the diff view here. I'm hoping to get all the functions moved tonight now that I know what most do and where they should go, so don't worry about it if you don't get to look at it soon. |
Beta Was this translation helpful? Give feedback.
-
|
I got the initial movement done. Almost every function that isn't directly callable by the user has been moved out of telekasten.lua. I did change my initial assessment about where a lot of the functions should go, so I think this will be a better layout than my first thoughts, but I'd be happy to take corrections or suggestions on any of it. I expect some of it will change as I dig deeper into the refactor, but we'll see how things go. This did introduce some circular requires as different modules became dependent on each other, which was a simple fix, but my test and work flow are clunky enough that it led to some rather minor commits, so I recommend squashing the PR to keep clutter out of the commit history. Speaking of PRs, I did make one for this first draft. As I mentioned in the PR, I still don't have a great testing library, so while I can confirm that the plugin loads without error and some basic functionality seems unchanged, it wouldn't hurt to test things out a bit. I do want to continue the refactor, but I think I need to address some features I need before I can start really using Telekasten, mainly just markdown link support. That's the sort of thing that's probably going to need to get merged into main, so it may be a good time to think about just what needs to be done before we can merge refact back into main. There's still plenty of refactoring work to continue afterwards, so it wouldn't be the end of the refact branch, but at this point, it seems best to use the refact branch as the base for it. |
Beta Was this translation helpful? Give feedback.
-
|
Awesome work! I will have a look at all that during the week-end. Thanks again for your involvement. |
Beta Was this translation helpful? Give feedback.
-
Hi all!
I finally managed to set aside a bit of time every week-end to work on Telekasten! Yay!
I plan to proceed as follows:
Hopefully by working on all this, I should simultaneously fix a few bugs and issues here and there. I will try to progress slowly and with caution in order to avoid any unfortunate breakage on your side. Ideally it would be nice to have some testers that would install the
refactorbranch and validate the changes I make before I merge them.So, to be clear, I do not plan on adding new features straight away. I think the first step is really to make the code more "maintainable" first so maybe we can get more people involved as well.
Beta Was this translation helpful? Give feedback.
All reactions