lcm-proj / lcm

Lightweight Communications and Marshalling
GNU Lesser General Public License v2.1
1.01k stars 393 forks source link

lcm-logplayer-gui: No .jlp with only defaults #530

Open judfs opened 1 month ago

judfs commented 1 month ago

If you just open and close a log, without clicking on any knobs, don't create a .jlp file.

google-java-format was suggested at https://github.com/lcm-proj/lcm/issues/420#issuecomment-1472374172 . I made a commit using it on the file I was working with first. Looking at the diff of 36a670eee9217152c2a65301d3dd69ba05d2e9f8 should be sane.

judfs commented 1 month ago

Once again I dared to use unicode. -encoding utf8 made the tests pass at least.

nosracd commented 1 month ago

I like intent of this PR. It seems a lot more sane to skip writing out a .jlp if the user does nothing but click the GUI window to close it.

One of the things I liked about the old behavior was this:

But in this PR, a .jlp isn't saved unless I disable a channel. What do you think about preserving the above behavior?

judfs commented 4 weeks ago

My motivation for this change is that it is inconvenient for additional files to be created when just playing back a log and doing nothing fancy. My team shares datasets as folders of LCM logs and random jlp files are undesired clutter. To this end, I would like not creating a jlp file when simply playing a log to be the default. "But in this PR, a .jlp isn't saved unless I disable a channel. What do you think about preserving the above behavior?": That was my entire goal.

I'd be interested in a opt in "Always make jlp" user preference.

I'd also be interested in a button to create a more proper index file for a log. I can imagine recording each of the following per channel:

logplayer-gui could then populate the filterlist from an index.

kyonifer commented 4 weeks ago

@judfs what are your thoughts about keeping the original behavior as the default, but adding a flag that enables the behavior you suggested above?

My preference is to keep the original behavior as the default to provide UX stability for users who have come to expect that behavior, but I definitely agree that there are use cases where the default is inconvenient. We could also consider environment variables instead of command line flags.

judfs commented 4 weeks ago

How about a dialogue on close?

Save log-player-gui session (.jlp)? [Yes] [No] Remember this for future logs [ ]

nosracd commented 3 weeks ago

In a side-channel, we discussed this a bit more. In all, the solutions we've considered are:

  1. Keep the existing behavior for continuity's sake. Add a new opt-in option to disable the existing behavior to improve the UX.
  2. Change the existing behavior to only write a .jlp when the user turns off a channel. Add a new opt-in option to preserve the previous behavior.
  3. Change the existing behavior to never write a .jlp. Add a new opt-in option to preserve the previous behavior.
  4. Add support for a .nojlp sentinel/marker file. If such a file exists alongside the log then the logplayer-gui will not write a .jlp.
  5. Move where the .jlp files are stored from beside the log file to somewhere in the user/system settings.
    1. We could use the filepath to the log to match the log setting to the file. Although this means if you move the log file you will lose your settings.
    2. We could hash the log file to match the settings to the log. This will introduce performance issues. It also means that there can no longer be multiple directories containing the log with different settings.