Opened 8 years ago

Last modified 2 years ago

#3212 assigned Enhancement

Add "Message Log" support to Qt client

Reported by: Longinus00 Owned by: mike.dld
Priority: Normal Milestone: 3.00
Component: Qt Client Version: 1.92
Severity: Normal Keywords: feature-disparity patch
Cc: taem@…, koboneil@…

Description

Tray icon still has Message Log even though it was removed from the client.

There are still references in ui_mainwin.h but I'm not sure if you want to keep those.

Attachments (6)

messagelog.patch (25.6 KB) - added by Koboneil 6 years ago.
messagelog-v2.patch (29.1 KB) - added by Koboneil 6 years ago.
0001-Add-message-log.patch (29.9 KB) - added by taem 5 years ago.
010-message-log-bits.diff (3.9 KB) - added by rb07 5 years ago.
Just the differences w.r.t. Taem's update, not including files not in Transmission repository.
messagelog-model.cc (3.6 KB) - added by rb07 5 years ago.
Mine uses some different key names.
3212-message-log.patch (30.5 KB) - added by mike.dld 3 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 8 years ago by Longinus00

diff --git qt/mainwin.cc qt/mainwin.cc
index 6703c7a..649b3fe 100644
--- qt/mainwin.cc
+++ qt/mainwin.cc
@@ -228,7 +228,6 @@ TrMainWindow :: TrMainWindow( Session& session, Prefs& prefs, TorrentModel& mode
     menu->addAction( ui.action_AddURL );
     menu->addSeparator( );
     menu->addAction( ui.action_ShowMainWindow );
-    menu->addAction( ui.action_ShowMessageLog );
     menu->addAction( ui.action_About );
     menu->addSeparator( );
     menu->addAction( ui.action_StartAll );

comment:2 Changed 8 years ago by charles

IIRC it was removed because there's no mechanism for getting the log messages via RPC. Maybe we should add that instead of removing these stubs.

comment:3 Changed 8 years ago by charles

  • Keywords feature-disparity added
  • Summary changed from Message Log shown in tray icon to Add "Message Log" support to Qt client
  • Type changed from Bug to Enhancement

comment:4 Changed 7 years ago by taem

  • Cc taem@… added

comment:5 Changed 6 years ago by gunzip

This long-absent feature would certainly be useful for the Qt client, perhaps similar to the logging now done by the daemon, i.e., write to syslog -or- a user-specified logfile.

Changed 6 years ago by Koboneil

comment:6 Changed 6 years ago by Koboneil

  • Cc koboneil@… added

Hi,

I attached a patch to implement this feature. You have to copy the {Red|Yellow|Purple}Dot.png files from macosx/Images/ to qt/icons/ .

comment:7 Changed 6 years ago by jordan

  • Milestone changed from None Set to 2.70

Thanks for this patch Koboneil!

This feature is a nice improvement over what Transmission has in 2.60, but the proposed new RPC call is "destructive" in a way that none of the other RPC calls are -- "get-messagelog" clears the list, so for exmaple if you had two standalone clients looking at the same session, each would get only a subset of the log messages.

A couple of possible refinements to this patch:

  1. We could add a time_t argument to get-messagelog s.t. libtransmission only returns messages newer than that timestamp. That way we wouldn't have to clear the messagelog on the session side.
  1. However even that could cause overlaps or gaps. We could add a sequential id field to the tr_msg_list struct, and then have get-messagelog behave like the first suggestion but with an id instead of a timestamp.

Koboneil, do you have an opinion on this?

Changed 6 years ago by Koboneil

comment:8 Changed 6 years ago by Koboneil

I modified the patch and added a sequential id field like you suggested jordan. The RPC call "get-messagelog" now requires an int argument.

To get all the messages : get-messagelog 0
To get all the messages from the 8th message (included) : get-messagelog 8

Since I don't know GTK and MAC OS programming, I couldn't fix these two clients to make them work with this new patch.

comment:9 follow-up: Changed 6 years ago by rb07

Just a few comments after testing this feature:

  • The filter is lacking an "All" option, and its confusing that the default (when you haven't touched the filter) is precisely "All", not the option showing, and there is no way to get back to "All".
  • I haven't seen memory problems, but I think that since all the messages seem to be stored somewhere it will be a problem for long running clients. The daemon is one we usually keep 24/7 for long runs, so the log will be huge at the daemon, and the same when Transmission-Qt (using a remote session) tries to display it. Am I wrong?
  • Doesn't it make sense to use a light-weight form of display, say Qt's QPlainTextEdit (or QTextBrowser) instead of the current table with column sorting, resizing, icons (you could use color instead of icons)?

comment:10 in reply to: ↑ 9 Changed 6 years ago by Koboneil

Replying to rb07:

Just a few comments after testing this feature:

  • The filter is lacking an "All" option, and its confusing that the default (when you haven't touched the filter) is precisely "All", not the option showing, and there is no way to get back to "All".
  • I haven't seen memory problems, but I think that since all the messages seem to be stored somewhere it will be a problem for long running clients. The daemon is one we usually keep 24/7 for long runs, so the log will be huge at the daemon, and the same when Transmission-Qt (using a remote session) tries to display it. Am I wrong?
  • Doesn't it make sense to use a light-weight form of display, say Qt's QPlainTextEdit (or QTextBrowser) instead of the current table with column sorting, resizing, icons (you could use color instead of icons)?
  • The filter behaves like all the filter I've seen and like the filter in the main window. You can get back to "All" by clearing the line edit manually or clicking on the button that appears when you type text in the line edit.
  • In libtranssmision, the messages are stored as a linked list and it's limited to 10000 messages. Transmission-Qt stores the message in a QList and it's not limited. Maybe should it be?
  • Why not use a table? I copied the message log window's ui of the Mac client, that's why I've used icons.

comment:11 follow-up: Changed 6 years ago by rb07

  • I wasn't referring to the text filter, I was talking about the combo box on the bottom left, the "Message class/severity/type filter". That combo is completely different from the main filters, it doesn't have an "All" (and the separator of course), and its the one that is confusing (i.e. its by default in "Info", but the log shows the non-existent "All").
  • As I said, I haven't seen memory problems, nor tested with a daemon, or long term... but 10,000 messages in one place, and Qt's handling (which is optimized internally) at the other, might increase memory use in ways that should be known (for instance, if the daemon starts using memory for those 10,000 messages it might be a problem for low resource installations, like routers).
  • In my opinion the table implementation is over-kill, I don't care what the Mac app uses, or what the KDE log viewer does, or whatever... its an opinion from using other tools to watch logs on many different applications and environments. What I tried to point out is that useless features (sort by column for instance) is just bloat.

By the way, there is a feature missing that is useful with log viewers: an option for enabling/disabling auto scroll.

comment:12 in reply to: ↑ 11 Changed 6 years ago by Koboneil

Replying to rb07:

  • I wasn't referring to the text filter, I was talking about the combo box on the bottom left, the "Message class/severity/type filter". That combo is completely different from the main filters, it doesn't have an "All" (and the separator of course), and its the one that is confusing (i.e. its by default in "Info", but the log shows the non-existent "All").
  • As I said, I haven't seen memory problems, nor tested with a daemon, or long term... but 10,000 messages in one place, and Qt's handling (which is optimized internally) at the other, might increase memory use in ways that should be known (for instance, if the daemon starts using memory for those 10,000 messages it might be a problem for low resource installations, like routers).
  • In my opinion the table implementation is over-kill, I don't care what the Mac app uses, or what the KDE log viewer does, or whatever... its an opinion from using other tools to watch logs on many different applications and environments. What I tried to point out is that useless features (sort by column for instance) is just bloat.

By the way, there is a feature missing that is useful with log viewers: an option for enabling/disabling auto scroll.

  • The combo box lets you set the maximum level of verbosity you want. So if you set Info you will get Error and Info messages. If you set Debug you will get all messages. Once again, I copied this behavior from the Mac Client.
  • I do know it will increase memory use with 10 000 messages. Perhaps the messages could be saved in an temporary file instead of memory?
  • Well, YOU think these features are useless, maybe others don't.
  • I agree about an option for enabling/disabling auto scroll.

comment:13 Changed 6 years ago by jordan

rb07, we're going to roll 2.70 fairly soon. Do you have an opinion on this patch for 2.70?

comment:14 Changed 6 years ago by rb07

jordan,

It works fine as far as I tested, which was minimal. Its an useful, but seldom used feature (like the daemon log).

I released Windows version 2.60, and 2.61 with this feature; so far nobody has commented anything.

I haven't tested a modified transmission-daemon + remote session in transmission-qt.

comment:15 Changed 6 years ago by jordan

  • Milestone changed from 2.70 to 2.80

comment:16 follow-up: Changed 5 years ago by taem

Hi,

I have refreshed the patch against the trunk. But this feature is not working. I think there is something wrong in getMessageLog() in rpcimpl.c. Can anyone please take a look at the patch and show me the right direction to fix it up?

Thanks.

Changed 5 years ago by taem

comment:17 in reply to: ↑ 16 Changed 5 years ago by rb07

Replying to taem:

Can anyone please take a look at the patch and show me the right direction to fix it up?

I'm attaching the differences I see between your changes and my working changes.

Changed 5 years ago by rb07

Just the differences w.r.t. Taem's update, not including files not in Transmission repository.

Changed 5 years ago by rb07

Mine uses some different key names.

comment:18 Changed 5 years ago by Koboneil

I took a look and the problem is the tr_variantDictFindList() function in session.cc at line 893. It returns false and I don't know why.

case TAG_MESSAGE_LOG: {
   tr_variant * messageLogList;
   if (tr_variantDictFindList (args, TR_KEY_messages, &messageLogList))
      emit messageLogReceived (messageLogList);
   break;
}

comment:19 Changed 5 years ago by jordan

  • Milestone changed from 2.80 to 2.90

comment:20 Changed 4 years ago by mike.dld

I may be missing something, but I'd rather remove this option from Mac and GTK+ clients as well (and those menu items from Qt client, which the ticket was originally about) instead of reviving it in Qt client. Using GTK+ client for several years now I never found a need in opening this dialog. I hardly imagine myself looking into log unless there is an issue, and since we aim to eliminate bugs instead of letting them prosper, the need for this option should be quite low.

What in my opinion needs to be done is enable logging to file for all those clients, as is the case for the daemon now. If someone wants to watch the logs, there is "Console" application on Mac, and *NIX users were never afraid of looking right into the file (although there are some GUIs/TUIs as well).

P.S. I couldn't think of any existing [standalone] program providing one with embedded log viewing. Transmission is about simplicity, right? Is there any particular reason for this kind of functionality here?

comment:21 Changed 4 years ago by x190

"there is "Console" application on Mac"

The Mac Client and its Message Log are a well-polished and highly functional pairing. No need to mess with success! :-)

"I hardly imagine myself looking into log unless there is an issue, and since we aim to eliminate bugs instead of letting them prosper, the need for this option should be quite low."

Transmission's logs also output many info and error messages that are helpful for diagnosing issues not directly related to code errors which, in themselves, will always be with us. The Message Log is an extremely useful and accessible tool for those of us trying to diagnose issues as well as help users diagnose their issues.

Last edited 4 years ago by x190 (previous) (diff)

Changed 3 years ago by mike.dld

comment:22 Changed 3 years ago by mike.dld

Attached is actualized version of the initial patch against r14545 with a couple of adjustments (automatic scroll to bottom and a bit redesigned dialog). Still it lacks GTK+ and Mac clients changes, and I'm uncertain as to whether it's a good idea to always keep last 10000 messages in memory.

comment:23 Changed 3 years ago by mike.dld

  • Owner changed from charles to mike.dld
  • Status changed from new to assigned

comment:24 Changed 2 years ago by mike.dld

  • Milestone changed from 2.90 to 3.00

comment:25 Changed 2 years ago by mike.dld

  • Keywords patch added
Note: See TracTickets for help on using tickets.