-
-
Notifications
You must be signed in to change notification settings - Fork 920
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
feat: added --follow option to hyprctl rollinglog
#6300
#6325
Conversation
f5417f9
to
17d034e
Compare
tbh I fail to see the use - one can always like... Maybe rollinglog should just do that? |
Using watch is kind of annoying and very limiting: I'll test this PR tomorrow, but from the description it does exactly what I wanted. |
@vaxerski can i ask for a review? |
As rollinglog can not follow the output like It would be good if rollinglog can follow the log, but I don't really like current implemention with keep sending log via the socket in a thread, I'd prefer a wrapper for tail which would be simple and efficient enough. And, perhaps, use something like |
Small detail: rollinglog works independently from any other logging methods. So we cannot use tail wrapper here. If the user does not want to write the log to a file, we cannot force him. PS PSS |
What do you mean with this? When I initially created the issue, my request was basically to make
This seems to be adding lots of complexity for something that could just be a wrapper for tail, like @rtgiskard says... |
@zjeffer You can disable logging in the configuration and still get it with the rollinglog command. The added --follow option preserves this logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additionally:
- clang-format needed
- no {} around short ifs (too many cases to label all)
- wiki mr needed
can u please explain more concrete? what exactly needs to be described? |
do you mean that it is necessary to remove {} for short Ifs? as far as I can see they are everywhere in MR |
the wiki mr yea thats all, just to let people know for ifs: we always if (a)
b(); in hyprland |
Done |
this MR will clash with #6380 and since that one is smaller can you please wait for until it's merged and then just fixup this one (use |
6380 is merged, can you please update the MR? |
@vaxerski done |
8da7c1a
to
9d8668e
Compare
conflict |
@vaxerski resolved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yabba dabba doo
Describe your PR, what does it fix/add?
Added --follow option to
hyprctl rollinglog
#6300
Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)
Also added
onlybuild
command to Makefile (it is annoying to completely rebuild whole project any time).Is it ready for merging, or does it need work?
Ready, working, tested.