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

Cleaner config.lua #19

Merged
merged 4 commits into from
Feb 10, 2024
Merged

Cleaner config.lua #19

merged 4 commits into from
Feb 10, 2024

Conversation

she3o
Copy link
Collaborator

@she3o she3o commented Feb 8, 2024

No description provided.

she3o added 3 commits February 8, 2024 10:24
    Use uv.hrtime(): Replaced vim.fn.reltime() with uv.hrtime() for better precision.

    Replace vim.fn.split with vim.split

    Utilized uv.fs_stat to check if a path is a directory and uv.fs_realpath to resolve symlinks.

    Replaced vim.fn.executable with uv.fs_access for checking executable permissions.
@she3o she3o requested a review from jalvesaq February 8, 2024 09:21
@jalvesaq
Copy link
Member

jalvesaq commented Feb 8, 2024

Github doesn't allow the merge due to selene finding errors.

@jalvesaq
Copy link
Member

jalvesaq commented Feb 8, 2024

I checked your branch out. It has loop errors. This means that module X requires module Y, which requires module X. I'm fixing this kind of issue by requiring module Y inside the function that uses it, not at the top of module X.

@jalvesaq
Copy link
Member

jalvesaq commented Feb 8, 2024

Could you move the line local edit = require("r.edit") from the top of config.lua to the function where it is used?

@she3o
Copy link
Collaborator Author

she3o commented Feb 9, 2024

I'll soon finish my work shift and be on it (Can it be done from Github mobile?).

Maybe I should just revert the second commit where I define r.edit and replace its references. I expect a lot of the functions will be moved around files anyway. Does that seem right?

@jalvesaq
Copy link
Member

jalvesaq commented Feb 9, 2024

I'll soon finish my work shift and be on it (Can it be done from Github mobile?).

I never used Github mobile, but there is no hurry.

Maybe I should just revert the second commit where I define r.edit and replace its references.

I think this would be enough for now.

I expect a lot of the functions will be moved around files anyway. Does that seem right?

Yes, at some point we may think about moving functions to more appropriate modules. Other tasks are:

  • Do grep -R '\\|' lua/ and fix patterns that don't work.

  • Document functions, or, at least, add the @param to allow linters to find functions called with invalid parameters.

@jalvesaq jalvesaq merged commit b72f799 into R-nvim:main Feb 10, 2024
1 check passed
@jalvesaq
Copy link
Member

It seems tha uv.fs_access requires the file's full path. So, it returns false when checking if "R" is executable.

@jalvesaq
Copy link
Member

As far as I know, Lua doesn't support patterns with |.

I will fix these issues in the next commit.

jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
jalvesaq added a commit that referenced this pull request Feb 22, 2024
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