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

implement ctags #16

Merged
merged 6 commits into from
Feb 27, 2025
Merged

implement ctags #16

merged 6 commits into from
Feb 27, 2025

Conversation

JMarkin
Copy link
Contributor

@JMarkin JMarkin commented Feb 16, 2025

Hello, I try to implement ctags for #14
Currently I copied/pasted all from symbols for draft. Main idea convert output ctags to lsp output.

Do you think to need to separate common parts into one library module? Like namu_ctags module move to namu_symbols and ctags as option, I don't know :)

Currently numa_ctags looks
image

@JMarkin JMarkin changed the title feat(ctags): copy/paste symbols to ctags feat(ctags): implement ctags Feb 16, 2025
@JMarkin JMarkin changed the title feat(ctags): implement ctags implement ctags Feb 16, 2025
@bassamsdata
Copy link
Owner

Thanks for the pr! I’m checking it out now and will get back to you. BTW, what operating system are you using? I might have some issues with ctags.

@bassamsdata
Copy link
Owner

bassamsdata commented Feb 16, 2025

Alright, I realized I had an older version of ctags on macOS, and after updating, everything worked fine.

Some initial thoughts:

  • Would it be possible to add a nil check for request as well? Right now, it throws an error the first time it's run. Maybe something like this:
  if request and request.is_closing and request.pid then
    -- Store the client and request_id
    state.current_request = {
      client = request,
    }
  else
  • Does ctags retain information about depth (parent/children relationships)? I noticed you defined a depth tree, but it doesn’t seem to be working as expected. If ctags does track depth, we might be able to debug where it gets lost when converting to selecta_items.

Do you think we should move common parts into a shared module? Like moving namu_ctags into namu_symbols and making ctags an option? Not sure. :)

For the final version, I think keeping it as an option makes sense. But for now, I’d prefer keeping it as a separate module. To make things cleaner, we could place it inside a namu_symbols/ctags/ directory. what do you think?

Also, I think symbolKindMap should have its own file, maybe inside namu_symbols/ctags/symbol_maps.lua, or another suitable name.

Really appreciate this, didn’t expect we’d get this working so quickly. Thanks a ton!

edit: some typo

@JMarkin
Copy link
Contributor Author

JMarkin commented Feb 20, 2025

@bassamsdata Sorry, I had much work couldn't complete it before. I fixed tree view and unnecessary error message.

Does ctags retain information about depth (parent/children relationships)? I noticed you defined a depth tree, but it doesn’t seem to be working as expected. If ctags does track depth, we might be able to debug where it gets lost when converting to selecta_items.

Maximum that I found is Scope, I don't know how it's supported by languages, but it looks like symbols tree.
image

Currently code has many duplicates between namu_ctags and namu_symbols modules, but I think refactoring would be better in other PR. What do you think?

@bassamsdata
Copy link
Owner

Sorry, I had much work couldn't complete it before.

No worries at all! Thanks a bunch for fixing it. I’ll test it out today, but before merging, I’ll tidy things up and push them to this PR (hope you don’t mind?). I also added some new features and need to make namu symbols more maintainable for handling multiple things, workspace symbols are in the works too!

@bassamsdata bassamsdata marked this pull request as ready for review February 27, 2025 20:12
@bassamsdata bassamsdata merged commit fb79b31 into bassamsdata:main Feb 27, 2025
2 checks passed
@bassamsdata
Copy link
Owner

Thank you a bunch for this PR! @JMarkin I made a few changes, mostly removing some repeated code, and I might clean up a bit more as I continue with the modular updates. Would you mind giving it a try after the latest update and let me know if everything’s working for you?

Also, I won’t be announcing it just yet since I still need to add the new features for normal symbols to ctgas.

@JMarkin
Copy link
Contributor Author

JMarkin commented Mar 1, 2025

@bassamsdata It works good, I don't find any problem in main branch, thank you.

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.

2 participants