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

On logging #95

Closed
097115 opened this issue Dec 17, 2020 · 6 comments · Fixed by #96
Closed

On logging #95

097115 opened this issue Dec 17, 2020 · 6 comments · Fixed by #96
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@097115
Copy link

097115 commented Dec 17, 2020

When using -o option, kirc seems to handle line breaks in a strange way: it inserts ^M followed by an empty line, like this (the log file opened in Vim):

line-breaks

Is this an intended behaviour? :)

v0.2.2 running on Linux.

@mcpcpc
Copy link
Owner

mcpcpc commented Dec 17, 2020

Each line from the raw IRC stream is terminated by a carriage return and a line feed character (frequently referred to as CR-LF). kirc prints the entire line from the stream without modifying it, including CR-LF. In vim, the ^M character is the carriage return character by default. So printing this character is intended behavior.

In most practical applications for output logging, the raw steam data will be parsed/modified; therefore, printing this character will cause little interference to the end-user. I am open to removing these characters though if you have particular example application in mind where this would cause problems. There also appears to to be a duplicate line feed (causing the blank line between printed lines), which can be fixed by removing the one also printed by the fprintf() function:

kirc/kirc.c

Line 397 in 9370902

fprintf(out, "%s\n", str);

If you need a quick short-term solution though to remove the ^M character, you can always use the following vim command (note that the ^M character below is inserted using Ctrl+V Ctrl+M):

:s^M$//

Hope this information helps.

@mcpcpc mcpcpc self-assigned this Dec 17, 2020
@mcpcpc mcpcpc added question Further information is requested enhancement New feature or request labels Dec 17, 2020
@kdkasad
Copy link
Contributor

kdkasad commented Dec 17, 2020

There also appears to to be a duplicate line feed (causing the blank line between printed lines), which can be fixed by removing the one also printed by the fprintf() function:

kirc/kirc.c

Line 397 in 9370902

fprintf(out, "%s\n", str);

I think this change should be implemented, since IRC commands already end with a newline. Plus, if someone is trying to use the log to debug an IRC command that does not end with a newline, they may think that it does since the print statement appends one, which could lead to problems.

@mcpcpc mcpcpc added bug Something isn't working and removed enhancement New feature or request labels Dec 17, 2020
@mcpcpc mcpcpc linked a pull request Dec 17, 2020 that will close this issue
@mcpcpc
Copy link
Owner

mcpcpc commented Dec 17, 2020

The extra new line feed has been removed per #96.

I think it makes sense to leave the CR-LF for the same argument you made above regarding debugging (since it is “expected” per the IRC protocol). I will merge the PR sometime tomorrow (after i have had a chance to consider a few additional changes) and 0.2.3 tarballs will be made available.

@097115
Copy link
Author

097115 commented Dec 18, 2020

@mcpcpc Thanks for the info!

And while we are here, one more thought: may be the log entries should also contain timestamps, what do you think? When browsing a log from time to time to check for the missed conversations, timestamps could help to understand where exactly are we right now... (I'm sort of trying to use the log file as you've suggested, grepping for #channel PRIVMSGs).

@mcpcpc
Copy link
Owner

mcpcpc commented Dec 18, 2020

@mcpcpc Thanks for the info!

And while we are here, one more thought: may be the log entries should also contain timestamps, what do you think? When browsing a log from time to time to check for the missed conversations, timestamps could help to understand where exactly are we right now... (I'm sort of trying to use the log file as you've suggested, grepping for #channel PRIVMSGs).

I was wondering when someone was going to request this. lol. Added per 20a7e3b and will be released with 0.2.3 later today.

Format will be as such:

[WEEKDAY] [MONTH] [HH:MM:SS] [YEAR]:[IRC RAW MESSAGE][CR-LF]

For example:

Fri Dec 18 13:34:55 2020::moon.freenode.net 372 kirc :- Thank you for using freenode!

@097115
Copy link
Author

097115 commented Dec 18, 2020

:)

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants