Enhance: added timestamps to log file. #984

gpalino · 2017-05-01T12:47:51Z

Using timeBuffer list, which is avaiable even when Show Time Stamps is turned off.
Both for html and txt log formats.

Using timeBuffer list, which is avaiable even when Show Time Stamps is turned off. Both for html and txt log formats.

vadi2 · 2017-05-01T12:51:47Z

Thanks for the improvement! I'll test it out.

SlySven

This is a useful suggestion - but I am not sure that everyone will want time stamps in logs of the main (or if applicable, I am not sure it is, in:) other consoles.

There are two separate things that can generate "logs":

the Lua command or main console bottom button that turns it on for the main console (next to the ones that controls the appearance of screen of the time-stamps that we are attempting to duplicate in a file here and the one that controls the recording of a file that can later be played back to "exercise"/"test" the whole setup's response to data from the MUD server {i.e. the "Replay" one})
manual selection of the text and then using the right-click (context) menu options to "copy" or "copy as HTML".

Does this change work on one or both things?

tl;dr; Is (or can) there (be) a way to control the inclusion of these time stamps?

SlySven · 2017-05-01T16:22:27Z

src/TBuffer.cpp

@@ -2400,7 +2400,11 @@ void TBuffer::log( int from, int to )
                }
                else
                {
-                    toLog = lineBuffer[i];
+                    if( !timeBuffer[i].isEmpty() )


Can't recall the exact details of the timeBuffer but is it empty on wrapped lines or filled with something other than a date-time stamp - possibly some '-'? It may be that we will need to insert 13 spaces to pad things out...

SlySven · 2017-05-01T16:30:31Z

src/TBuffer.cpp

@@ -3171,6 +3175,14 @@ QString TBuffer::bufferToHtml( QPoint P1, QPoint P2 )
    QString fontStyle;
    QString textDecoration;
    bool firstSpan = true;
+    if( !timeBuffer[y].isEmpty() )
+    {
+        firstSpan = false;


Remember firstSpan being true initially is needed to ensure that the CSS is present in the first  in the created HTML file and to avoid the insertion of a \ in the first iteration around the for(...) that follows here... 😟

Actually, further examination suggests this is correct after all! 😀

Example of incorrect alignment: 14:21:34.413 (mapper): The games map was updated - you should update yours! Go to Settings -> Mapper tab and -----click on the button there. Once you've updated, click here to remove the reminder.hide

gpalino · 2017-05-01T21:50:21Z

I've added possibility to turn on/off timestamp logging in Settings -> General; hopefully correctly.

I am not familiar with recording & replaying, so please test it.

"copy as HTML" will probably not work as expected, because depending on Setting related to log files, the timestamp will be either included or not included in html snippet.
What do you think

Is this acceptable?
Should be timestamp snippets forbidden for "copy as HTML"
Should we use setting "Show Time Stamps" (one of bottom buttons) to decide whether to include timestamps or not?

I checked the generated HTML logs with https://validator.w3.org/
As far as I've seen, my changes brought no issues (tested shortly with achaea and aardwolf).
There is just different kind of bug - missing </div></body></html> at the end

vadi2 · 2017-05-02T03:29:20Z

I think the "Show time stamps" bottom button toggling "copy as HTML" is a good idea, that way people can quickly change if they'd like timestamps in a particular paste or not.

vadi2 · 2017-05-02T03:44:55Z

@gpalino what you've got so far looks great.

adayoung · 2017-05-02T05:27:40Z

I feel it should copy what is shown on the screen. So if timestamps are visible, it shouldn't be a surprise when the html log also includes timestamps 😁 Thank you for the enhancement!

The setting "Show Time Stamps" is used to determine if timestamps are included or not.

gpalino · 2017-05-02T14:57:05Z

Now it should be finished from my point of view, I've fixed copy HTML behaviour.

vadi2 · 2017-05-02T19:09:07Z

Could you do the same for plain copy - if timestamps are visible, they're added to what is copied?

3.1 release will be tomorrow, and side from this, it looks good from my pov. If we could get this in and approvals @SlySven @ahmedcharles then I'll be happy to get it into the release as it has a small regression footprint.

gpalino · 2017-05-02T21:32:37Z

I've extended also void TTextEdit::copySelectionToClipboard(), to add timestamps to plain copy.
I consider it could be annoying sometimes (e.g. I want to quickly copy-paste text from console to input line, regardless of Show Time Stamps setting), so I made a compromise. Timestamps are included only when selecting more than one line of text.

vadi2

Good thinking, works great. Thanks! 💯

ahmedcharles · 2017-05-03T11:02:21Z

I'm good with @SlySven and @vadi2 reviewing.

vadi2 · 2017-05-03T15:13:02Z

Merging this in as @SlySven's review comments have been addressed - would like this in the 3.1 release.

A recent PR: Mudlet#2365 removed code which was rendered ineffective by an implementation error in: Mudlet#984 however it causes an error which has just been raised in: Mudlet#2367 This PR should fix the issue and it also fixes another bug that I can see in that underlines are not being reproduced in the HTML files because of a typo in that part of the style detail for `` entries which used "text-decoration: undeline" instead of: "text-decoration: underline"! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

A recent PR: #2365 removed code which was rendered ineffective by an implementation error in: #984 however it causes an error which has just been raised in: #2367 This PR should fix the issue and it also fixes another bug that I can see in that underlines are not being reproduced in the HTML files because of a typo in that part of the style detail for `` entries which used "text-decoration: undeline" instead of: "text-decoration: underline"! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

Enhance: added timestamps to log file.

ac0fc59

Using timeBuffer list, which is avaiable even when Show Time Stamps is turned off. Both for html and txt log formats.

vadi2 added the 1 - Ready label May 1, 2017

vadi2 requested review from vadi2, ahmedcharles and SlySven May 1, 2017 12:50

SlySven reviewed May 1, 2017

View reviewed changes

gpalino added 2 commits May 1, 2017 22:59

Added configurable option "Add timestamps at the beginning of log lines"

ad6900c

Corrected timestamps when using "copy HTML" functionality.

550e16e

The setting "Show Time Stamps" is used to determine if timestamps are included or not.

Added timestamps also to plain copy.

6890174

vadi2 approved these changes May 3, 2017

View reviewed changes

vadi2 merged commit d332196 into Mudlet:development May 3, 2017

SlySven mentioned this pull request Feb 23, 2019

Remove dead code for html copy starting in the middle of the line #2365

Merged

SlySven mentioned this pull request Feb 23, 2019

BugFix: add missing "" in timestamped copy-to-HTML & log files #2369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance: added timestamps to log file. #984

Enhance: added timestamps to log file. #984

gpalino commented May 1, 2017

vadi2 commented May 1, 2017

SlySven left a comment

SlySven May 1, 2017

SlySven May 1, 2017

gpalino commented May 1, 2017 •

edited

vadi2 commented May 2, 2017

vadi2 commented May 2, 2017

adayoung commented May 2, 2017

gpalino commented May 2, 2017

vadi2 commented May 2, 2017

gpalino commented May 2, 2017

vadi2 left a comment

ahmedcharles commented May 3, 2017

vadi2 commented May 3, 2017

Enhance: added timestamps to log file. #984

Enhance: added timestamps to log file. #984

Conversation

gpalino commented May 1, 2017

vadi2 commented May 1, 2017

SlySven left a comment

Choose a reason for hiding this comment

SlySven May 1, 2017

Choose a reason for hiding this comment

SlySven May 1, 2017

Choose a reason for hiding this comment

gpalino commented May 1, 2017 • edited

vadi2 commented May 2, 2017

vadi2 commented May 2, 2017

adayoung commented May 2, 2017

gpalino commented May 2, 2017

vadi2 commented May 2, 2017

gpalino commented May 2, 2017

vadi2 left a comment

Choose a reason for hiding this comment

ahmedcharles commented May 3, 2017

vadi2 commented May 3, 2017

gpalino commented May 1, 2017 •

edited