feat: use short "GPU" name in memory widget if there's only one

[gepbird]

Description

When you have a single GPU, you don't need to see it's full name, "GPU" is fine.

Issue

In a smaller terminal window the legend for the memory widget was not visible, because the GPU name tried to take up too much space.

When resizing the terminal window or the widget, it's better. Reading it is still a bit harder, as the TODO comment suggests in the code:

Testing

If relevant, please state how this was tested. All changes must be tested to work:

If this is a code change, please also indicate which platforms were tested:

• [ ] Windows
• [ ] macOS
• [x] Linux

Checklist

If relevant, ensure the following have been met:

• [x] Areas your change affects have been linted using rustfmt (cargo fmt)
• [x] The change has been tested and doesn't appear to cause any unintended breakage
• [ ] Documentation has been added/updated if needed (README.md, help menu, doc pages, etc.)
• [x] The pull request passes the provided CI pipeline
• [x] There are no merge conflicts
• [ ] If relevant, new tests were added (don't worry too much about coverage)

[ClementTsang]

Hmm. Not 100% sure about this one at first glance, might be fine with this being the default setting with the old setting being an option. Let me think about it for a bit and get back to you.

Slightly related, I was also going to add a fix to limit the length of the GPU name in the legend based on width + align it better, but I do agree the default behaviour being just "GPU" for a single GPU is probably fine.

[jamartin9]

Slightly related, I was also going to add a fix to limit the length of the GPU name in the legend based on width + align it better

We used to use a 'short name' that was derived from the last two parts of the string ie '1050 Ti'. While not really conforming to the 3 letter convention it was less harsh on the legend sizing with multiple gpus. Edit: amd names may need changed to fit conventions too.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 42.08%. Comparing base (d55e1a6) to head (5437c1b).

Files with missing lines	Patch %	Lines
src/canvas/widgets/mem_graph.rs	0.00%	4 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1791      +/-   ##
==========================================
- Coverage   42.09%   42.08%   -0.01%     
==========================================
  Files         115      115              
  Lines       16208    16211       +3     
==========================================
  Hits         6823     6823              
- Misses       9385     9388       +3     

Flag	Coverage Δ
macos-14	37.34% <0.00%> (-0.01%)	:arrow_down:
ubuntu-latest	43.64% <0.00%> (-0.01%)	:arrow_down:
windows-2022	37.70% <0.00%> (-0.01%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gepbird]

Hmm. Not 100% sure about this one at first glance, might be fine with this being the default setting with the old setting being an option. Let me think about it for a bit and get back to you.

What do you think, is the current approach fine? Personally I like that it aligns well with just 3 characters, but I understand having a special case for single GPUs is not the best.

We used to use a 'short name' that was derived from the last two parts of the string ie '1050 Ti'. While not really conforming to the 3 letter convention it was less harsh on the legend sizing with multiple gpus. Edit: amd names may need changed to fit conventions too.

By the way, why was that removed? I'm happy to bring it back for the multiple GPUs case.

[ClementTsang]

Sorry for the delay - just to answer the questions:

What do you think, is the current approach fine? Personally I like that it aligns well with just 3 characters, but I understand having a special case for single GPUs is not the best.

To be honest I think it's fine to have a special case for single GPUs - but it might just make more sense to make it so there's an option to choose either short simple GPU names (GPU0, GPU1) or the longer ones. I would be fine with making short GPU names the default as well.

In a future PR I can also make it so the longer GPU names don't clash as badly even if used.

By the way, why was that removed? I'm happy to bring it back for the multiple GPUs case.

Don't think it was an active choice as far as I'm aware?

[gepbird]

Oops I missed your previeus comment.

To be honest I think it's fine to have a special case for single GPUs - but it might just make more sense to make it so there's an option to choose either short simple GPU names (GPU0, GPU1) or the longer ones. I would be fine with making short GPU names the default as well.

That also sounds reasonable, in that case I think the RAM and SWP should have a 1 character padding to make it aligned.

In a future PR I can also make it so the longer GPU names don't clash as badly even if used.

For multiple GPUs that sounds fine too (because the user may not know which is GPU0 and which is GPU1). If I had multiple GPUs personally I wouldn't be a fan of this, because I use bottom in a scratchpad (a smaller pop up terminal) and on my layout the memory takes 50% width and ~30% height, the legend previously took up a reasonable space from the graph.

Don't think it was an active choice as far as I'm aware?

That's good to hear it was likely not intentional :)

I'm okay with accepting this for now, though I might change the behaviour of this later.

I don't really plan to make edits in this PR (I'm not too confident in this codebase and Rust + time limitations). Feel free to push changes here or change it later.