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

fix: broken log file path for NullLsLog #16

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

eshepelyuk
Copy link
Collaborator

@eshepelyuk eshepelyuk commented Nov 14, 2023

NullLsLog command is currently broken and does nothing.

@eshepelyuk eshepelyuk marked this pull request as draft November 14, 2023 18:03
@eshepelyuk eshepelyuk changed the title fix: log file path for NullLsLog command [WIP] fix: log file path for NullLsLog command Nov 14, 2023
@mochaaP
Copy link
Member

mochaaP commented Nov 14, 2023

Is this intended? It's a log file, not a folder

@eshepelyuk
Copy link
Collaborator Author

eshepelyuk commented Nov 14, 2023

Is this intended? It's a log file, not a folder

yes it is.
the real file log name has no extension, i.e. ~/.cache/nvim/null-ls

@jakenvac
Copy link
Collaborator

Is this intended? It's a log file, not a folder

yes it is. the real file log name has no extension, i.e. ~/.cache/nvim/null-ls

Would a more appropriate fix be to ensure that the actual log files are created with the .log extension in this case?

As an aside, it would be really helpful if you could update the PR message to address the problem you are trying to solve.

@eshepelyuk
Copy link
Collaborator Author

Is this intended? It's a log file, not a folder

yes it is. the real file log name has no extension, i.e. ~/.cache/nvim/null-ls

Would a more appropriate fix be to ensure that the actual log files are created with the .log extension in this case?

The PR is still in draft.

@eshepelyuk eshepelyuk force-pushed the fix/null-ls-log branch 2 times, most recently from 55065ed to 866e7a3 Compare November 15, 2023 08:56
@eshepelyuk eshepelyuk changed the title [WIP] fix: log file path for NullLsLog command fix: broken log file path for NullLsLog Nov 15, 2023
Signed-off-by: Ievgenii Shepeliuk <eshepelyuk@gmail.com>
@eshepelyuk eshepelyuk marked this pull request as ready for review November 15, 2023 09:00
@eshepelyuk eshepelyuk requested a review from mochaaP November 15, 2023 09:00
@jakenvac
Copy link
Collaborator

jakenvac commented Nov 15, 2023

The PR is still in draft.

The perfect time to discuss proposed changes! 😄

@mochaaP
Copy link
Member

mochaaP commented Nov 15, 2023

🤔 I still didn't get it. What's precisely the bug here? Could you list expected behavior and the actual behavior?

@mochaaP
Copy link
Member

mochaaP commented Nov 15, 2023

Oh, I see 🙈. Will check it now!

Copy link
Member

@mochaaP mochaaP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing log file is handled by plenary, need further investigation

@eshepelyuk
Copy link
Collaborator Author

Writing log file is handled by plenary, need further investigation

What sort of investigation ? What are you trying to find, maybe I can answer those question immediately ?

@mochaaP mochaaP merged commit 0f38652 into nvimtools:main Nov 17, 2023
3 checks passed
@eshepelyuk eshepelyuk deleted the fix/null-ls-log branch November 17, 2023 05:51
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