Allow unlimited length user messages while still truncating their display by default. #694
Conversation
|
@barjac, might want to try this and make sure it doesn't introduce any weird problems. (I'm currently traveling for another week or so, so my availability for testing is limited.) |
|
Initial testing seems to show this is working here, I will play around some more and see how it goes. |
Likewise. I'm finding the display of the full message tooltip is a bit intermittent when hovering over the message, not sure whether this could be made more persistent, as in once displayed hold it for a few seconds even if the mouse moves a little. Also: This could be very annoying to users on earlier versions for whatever reason. Could the messages be truncated when sent to users with detected incompatible versions (maybe impossible but just thinking out loud ;) To clarify the above I noticed yesterday when you and Mooneer were putting up test messages that my older version was running out of screen width. No doubt when this feature is released some people will push the 'no width limit' boundaries and wipe out all columns to the right of the Msg column for 'older version' users. |
|
Something I have just noticed is that I need to highlight the row before I can see the full tooltip, and yes it's a bit picky about displaying the tip consistently. I would guess that is DE-dependent in some way, or wxWidgets gets its fingers in the pie too. Right click to save another's message to the clipboard sounds a useful feature even if something I might only use occasionally. |
The latest commit should hopefully fix this. Or at least it does on macOS, anyway. (Weirdly, I didn't see this issue on Windows before the latest commit. Not sure why.)
Will need to think about this more as I'm not sure it'd be obvious to other users that right-clicking that column copies text to the clipboard.
I think it might be possible to truncate on the server side. However, the version field is actually free-form text (albeit in a known format for the official FreeDV clients), not to mention that what's currently a single line to broadcast that message will likely turn into more involved logic. We can monitor how quickly people upgrade to the next release and if it turns out that it's taking a while, we can revisit this. |
|
The flicker fix on my Linux system is a bit better, but still requires the row to be highlighted. Not sure how many tools wxWidgets provides for this sort of control. |
Shouldn't need the row selected anymore as of the latest commit. |
Seems better, however now showing me as Rx only... Which turns out to be various audio devices disappearing or being deselected on the transmit tab of the audio options. Odd, but now fixed again. Can't see how the portaudio error change could do that. |
I was thinking about a context menu with option to save to clipboard. The 'hover only' is now reliable here - much better than having to select first :) |
|
Added context menu to allow copying to the clipboard. Note that this menu will only appear when hovering over a message. |
|
The sequence I get is: Hover pointer over message, tooltip appears. Right click, tooltip disappears. Second right click, row highlights. Third right click shows context menu. Message is correctly copied to clipboard on clicking the menu item. There's something a bit odd about the way this is handled in wxWidgets, or at least that's what seems to be the case. |
I did a bit more tweaking and I think this is fixed now. |
|
Yes, definitely fixed, thanks @tmiw I looked at the patch, I have no idea what you did or how you know to do it. Very impressed! |
No not for me at #2cd00c0. :( I just made a video showing this. |
|
That's unexpected. I have checked again, I don't see this any more. GNOME Wayland 45 on Fedora 39. I know that you use KDE Plasma @barjac |
|
I just installed Plasma on my Fedora 39 VM and that seems to behave the same as GNOME for me, FWIW. Maybe I'm missing something? |
|
I was made aware yesterday that @barjac uses Mageia. Unfortunately my normal test machine is in the shop and I only have access to an ARM one at the moment, so further testing will have to wait (unless there is in fact an aarch64 Mageia release that I missed when looking around on the website?) That said, I wouldn't think there'd be that much of a difference between different distros when using the same desktop environment. Might be worth trying a resync and a full rebuild and making sure there's nothing old still sticking around? |
|
Nothing gets 'left around' here. I always build test 'packages' in the same chrooted environment that Mageia uses for official builds. Every test package is installed using the distro package manager. The build chroot only has a minimal package set plus the build requires for the package being built. I have tested builds for both the current stable Mageia-9 x86_64 and also unstable development version Mageia-10 x86_64. Both behave exactly the same. Mga9 has Plasma-5 Mga10 has Plasma-6. I have realized that if 'right click hold' is used, the 'Copy message' box stays as long as the mouse button is depressed. (This does copy the message) Maybe this helps to diagnose? I have just built a package of this branch for aarch64 as I do have a Mageia-9 system on a RPi4. On the RPi4 using LXQt desktop, a short right click does show the 'Copy message' box permanently until it is either left clicked or a left click elsewhere is made. Left clicking the box does save the message to the clipboard, so it appears to work correctly on the RPi using LXQt D.E. My only tentative conclusion so far is that the problem is related to x86 builds, irrespective of D.E. |
|
Here is the list of shared objects for freedv on my Fedora 39 system: ldd /usr/bin/freedv Maybe compare and see if there are any .so version differences relative to your distro. The other question is how often does Mageia update packages? I have just noticed that plasma 5 is about to get a new bugfix version on Fedora, the packages appeared today in pending->updates-testing. |
|
Apparently aarch64 Ubuntu 22.04 and KDE Plasma was the right combo. Hopefully the changes I just pushed help; they seem to for me. |
Still working for me here, the only noticeable change is that the 'Copy message' button appears slightly to the left of the message instead of to the right, which is what the commit says in the comment. It's actually slightly easier to click without moving the mouse. Over to @barjac :) |
|
That has fixed it - it's now working as tyrbiter described. Tested OK so far in Mga9 x86_64 and Mga9 aarch64. |
--snip-- I got about half way through checking and there are quite a few differences. In Fedora I don't see libsioclient, libnotify, libicudata, libicuuc nor libpixman. In Mageia I don't see libgsm, libLerc nor libreadline. There may be others but that is as far as I got manually, I should have written a script to do a proper full comparison but it didn't seem worth the effort at the time :\ Thoughts anyone? |
A few comments: libsioclient is not (yet) available as a Fedora rpm so it gets downloaded and built during the freedv build, maybe one day @hobbes1069 will fix that up, I think he had some problems with the Fedora build system. I know you don't do that in your systems. libnotify and libnotify-devel are installed here but not linked in during the build, I don't think that it's a buildrequire either. libicudata.so. is installed, again appears not to be mentioned in the spec file, the same is true for libicuuc.so. Not sure exactly what they do related to freedv
Distros differ I suppose. |
OK to merge, then? Will another issue get created for Mga10 if it ends up being something FreeDV related? |
Yes - it is fine in stable systems and we expect issues in Cauldron. |
|
Following IRC chat about libasan errors these are terminal outputs from building and running of freedv. Output from freedv build_linux.sh Output running freedv and starting modem |
|
I created drowe67/codec2#43 for the build error. This looks like it's a failure in the test programs and not in the library itself. As for the other one, the following makes me think it's some sort of locale issue: Might be worth running |
|
Thanks for creating the codec2 bug. |
Hmm, I wonder if this change I did as part of the Codec2 PR I just opened for the build issue you reported will resolve that. What happens if you disable multiple receive and set FreeDV to something other than 800XA? |
|
I tried to build freedv using build_linux.sh edited to use codec2 ms-libasan-crash-fix branch but it aborts with dynamic-stack-buffer-overflow using:
Testing with a previous successful build_linux.sh build does run the modem without crash. |
Try that codec2 build again. I was testing without LPCNet compiled in before and missed that failure. |
|
Yes it now builds OK. Running with radio off and LC_ALL=C it continues to run: Running with radio on and LC_ALL=C it crashes: If I run it with radio off I can stop and start the modem at will without a crash. I get the hamlib communication timeout messages but still no crash. If I then stop the modem, turn on the radio and start the modem the radio beeps indicating that it is communicating but then it crashes.: All the above with multi RX on. 700E TX mode. OK bed time here now :) ZZzzz |
|
One more try with the latest? I believe I was able to get all the tests that were failing with libasan enabled to pass. |
|
I don't think there is any change. Run: The modem runs for longer after starting it with LC_ALL=C maybe 500ms. With multi RX off: Without LC_ALL=C it crashes: |
|
Can you create a separate issue in this project for the memory leaks? I think those are definitely in the application and not Codec2. Also, try the latest from Codec2 again and see if that helps at all? |
|
No real change. With radio on in multi RX mode it crashes: I have the other outputs from all 4 cases if they are needed. In view of David's comment in codec2 PR#43 do you still want the leak bug reported? |
Can you duplicate the crash without AddressSanitizer built in? If so, how often?
Yeah, some of the leaks are indeed in this project and not on codec2. For example: |
No, I have not seen any crashes recently using master which I normally run from locally built packages.
OK I will do it ASAP. |
Sorry it didn't happen, just too busy with family etc. over the holiday and I now see that you already fixed it. :) |
This PR lifts the 15 character limit on user messages, allowing users to send much longer messages as desired. However, display of the longer messages is still truncated to 15 characters by default (to keep the behavior the same for smaller displays); to view the full message, you need to select the user in the list and hover over the column. For example: