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

Initial function movement #332

Merged
merged 18 commits into from
Jun 15, 2024
Merged

Conversation

TheTaoOfSu
Copy link

Proposed change

This PR doesn't make any changes to functionality. It only rearranges existing functions by placing them in more suitable files and updating any calls to them to properly address them. It also introduced and then resolved several loop requires.

It's quite possible that some of these functions have been moved to a less-than-ideal new home. I did adjust many of my initial assessments, but I'm very open to corrections or suggestions.

My testing and development workflow is disjointed enough that I had to push some pretty minor commits to move and test updates, so this is probably best squashed to a single commit. I'll try to change that soon to de-clutter the merged history.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Code quality improvements to existing code or addition of tests
  • Documentation update

Additional information

This PR does not directly relate to any known open issues. However, it does relate to discussion #239.

Checklist

  • I am running the latest version of the plugin.
  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • The code has been formatted using Stylua (a .stylua.toml file is provided)
  • The code has been checked with luacheck (a .luacheckrc file is provided)
  • The README.md has been updated according to this change.
  • The doc/telekasten.txt helpfile has been updated according to this change.

As mentioned in my last PR, I do not have a suitable vault for in depth testing. I have a basic one with a couple notes, and it seems to work fine there, but I can't test very thoroughly this way.

@lambtho12
Copy link
Member

Awesome! I will try to check all this during the week-end.
In the meantime, could you resolve the conflict first?

@TheTaoOfSu
Copy link
Author

Oh, yes, sorry, forgot about that. I got it updated.

@TheTaoOfSu
Copy link
Author

I thought that comment style used in ujisati's commit #331, using tags like @param and @return, was a better way to structure the comments I'd already made, so I reworked them in that style. I didn't touch any of the others. It seems this may be LDoc/LuaDoc style for comments. If you'd prefer another style or to just avoid choosing a style at the moment, I don't mind reverting, but I figured even if you don't want to use the documentation generator, conformance means using a very consistent style with clear separation and consistent definition for things that get @tags.

@lambtho12
Copy link
Member

First of all, sorry for the delay. I was fairly busy starting a new job. I will try to review and merge faster next times.
Second, thank you very much for moving forward with the refactoring.

For the comment styles, I do not really mind, but like you I agree that having a coherent style through the entire code is important. The one you implemented in the last commits is really clear and we can go with that. I have never used documentation generator for lua projects but if the current style allows for it, that is great for the future.

@lambtho12 lambtho12 merged commit 311b1a1 into nvim-telekasten:refact Jun 15, 2024
2 checks passed
@ujisati
Copy link

ujisati commented Jun 16, 2024

@lambtho12 @TheTaoOfSu thanks for considering #333. I'm not super familiar with lua but it seems like https://luals.github.io/wiki/ is a standard based on github stars.

It has its own format: "LuaCATS (Lua Comment And Type System) annotations, which are based off of EmmyLua annotation", but they are not cross-compatible with emmylua.

Lua-ls has a documentation generator as well.

I'm happy to contribute docs, I want to get a better understanding of the code to help me migrate off of Obsidian, and maybe try to replicate some of that feature set eventually. But lua ls seems to have the best language support so im gonna keep documenting with that unless ya'll tell me not to.

@ujisati ujisati mentioned this pull request Jun 17, 2024
11 tasks
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

Successfully merging this pull request may close these issues.

3 participants