Statusbar update 212 952 1189 #1200

SchweizS · 2020-04-23T18:21:41Z

This change addresses item

This adds:

all buttons can be hidden (Configure buttons in the status bar #212)
added launch button (Run in terminal in the bottom bar #1189)
added option to disable coloring of CTest button (Status bar text should be theme-able #952)
added option to truncate name of selected kit (Configure buttons in the status bar #212)
added option to only show build icon (Configure buttons in the status bar #212)
added option to reduce size of extension status (CMake: Debug: Ready => Debug: Ready) (Configure buttons in the status bar #212)
added tooltip to the build button

Changes:

CTest button is not colored by default.
LaunchTarget is now in brackets [target] similar to build target
Tooltip of debug, launch and build buttons now shows the target

This does not address:

build and debug target are still separate (Configure buttons in the status bar #212)
no custom coloring (Status bar text should be theme-able #952)

Notes:

~~as far as i can tell hideDebugButton is never called. maybe the entries related to that can be completely removed.~~
there seems to be a bug which causes the workspace selection not to show up when a workspace is created by adding a second directory.
visibility of buttons can be one of
- default: current behaviour. Label is completely visible
- compact: buttons only show minimal information or get truncated
- icon: only icons are visible. buttons without icons are hidden
- hidden: button is hidden

bobbrow

This is pretty cool and I'm glad that there is interest out there to address this. We've been debating here what we should do about the status bar because it does take up a lot of space. We've actually been thinking about moving this out of the status bar completely and into its own view (i.e. perhaps sharing space with the "outline view" panel as a separate section). We could then just leave a single icon on the status bar that could act as a shortcut to the panel. We haven't fleshed out the details, but that seemed like an option we wanted to explore.

That said, I think this PR gives people a lot of control over how the status bar looks and is a good interim solution in the event we want to do the aforementioned work. However, my overall feedback for this PR is that this might give a little too much control and result in some settings bloat. It was somewhat cumbersome to change the settings for each button. I would expect that most people would be happy with a simple setting to control all of the buttons. "text"/"icon"/"hidden". "short" is mostly a problem for Kits with long names and I think it's not essential to try to solve that. Icon + Tooltip would be sufficient, I think. If we consolidate the settings, then in the event that we do move the controls out of the status bar, that's fewer settings for us to deprecate.

I'd like to know what your thoughts are in this regard. I'm not saying we won't accept this PR. I just wanted to share our thoughts before we proceed and see how that affects the path we take to clean up the status bar.

Thanks for tackling this!

src/status.ts

package.json

src/status.ts

SchweizS · 2020-04-25T14:16:21Z

This is pretty cool and I'm glad that there is interest out there to address this. We've been debating here what we should do about the status bar because it does take up a lot of space. We've actually been thinking about moving this out of the status bar completely and into its own view (i.e. perhaps sharing space with the "outline view" panel as a separate section). We could then just leave a single icon on the status bar that could act as a shortcut to the panel. We haven't fleshed out the details, but that seemed like an option we wanted to explore.

I like that idea, but i think the statusbar should still remain an option.
The statusbar allows to see some settings at first glance, without having to change to a different view.
This is in my opinion a big win in comparison to other options.

If you add the new view, you could just set the default for the statusbar to hidden and add the new button.

That said, I think this PR gives people a lot of control over how the status bar looks and is a good interim solution in the event we want to do the aforementioned work. However, my overall feedback for this PR is that this might give a little too much control and result in some settings bloat. It was somewhat cumbersome to change the settings for each button. I would expect that most people would be happy with a simple setting to control all of the buttons. "text"/"icon"/"hidden". "short" is mostly a problem for Kits with long names and I think it's not essential to try to solve that. Icon + Tooltip would be sufficient, I think. If we consolidate the settings, then in the event that we do move the controls out of the status bar, that's fewer settings for us to deprecate.

I had the same issue, but i think i have a solution for that.

With that you have both: An easy way to change all buttons and (kind of) hidden settings for each individual button. It might be a good idea to add some descriptions if this gets approved.

bobbrow

Sorry, I'm not quite done yet, but I do want to provide some incremental updates/status on this PR so we can work in parallel.

src/status.ts

src/extension.ts

package.json

src/status.ts

bobbrow

This is looking really awesome! I'm still testing, but I have a few remaining comments.

src/status.ts

package.json

added default icon for CTest (beaker)

- renamed type to visibility - added toplevel localization

added tooltip to BuildTargetName

SchweizS · 2020-05-04T10:17:25Z

with the current changes i think we could also remove the label of the Build-button.
@bobbrow

i also updated the first post to reflect the changes

- fixed access - reordered function to reflect access - renamed member to reflect access

renamed ActiveFolderButton to WorkspaceButton

SchweizS · 2020-05-05T06:36:13Z

the last commits contain minor improvements to compact mode and an overall cleanup.

macdems · 2020-05-05T08:13:17Z

This is a great improvement to a serious cmake-tools drawback. The ability to turn-off selected buttons is something that was strongly missing. For example, I need to be able to select a variant, however I don't want to touch the kits (and accidentally clicking the 'select kit' statusbar button can result in half-an-hour recompilation of the whole project, so being able to hide it is important to me). Please accept this PR for the next release.

bobbrow · 2020-05-05T16:59:42Z

We're almost done iterating on this. It will be in the 1.4 release. It's in the "fit and finish" phase right now. I am going through all of the tooltips and settings combinations (there are a lot!) looking for consistency issues. I have a few comments pending right now and plan to submit my feedback within the next day or so.

bobbrow

I'm not done checking the tooltips yet as I had a full day of meetings today. But I did want to post this feedback now in case you had other changes pending. I wrote most of this last night and removed the comments related to stuff you pushed this morning.

src/status.ts

package.nls.json

src/status.ts

bobbrow

I think I got most of the UI stuff tested and added comments to the review. I'll do one last check after your next push and then I think we'll be good to go!

Thanks again for all of your hard work on this! I'm super excited to get this in!

src/status.ts

bobbrow · 2020-05-07T16:39:02Z

src/status.ts

+  protected getTextShort(): string { return this.getTextNormal(); }
+  protected getTextIcon(): string { return ''; }
+
+  protected getTooltipNormal(): string|null { return this._tooltip; }


Tooltip issues:

default mode

Button comment

Status Remove the status from the tooltip. It should just say "Click to select the current build variant"

compact mode

Button comment

Workspace I don't think we need to show the folder name in the tooltip because we don't truncate the text.

Status I think the status should come before the command text. Can you switch the order?

Kit I think the active kit should come before the command text. Can you switch the order?

Build The text should start with "CMake:"

Build label I don't think we need to show the selected build in the tooltip because we don't truncate the text.

Launch label The text should start with "CMake:"

CTest I don't think we need to show the button text in the tooltip. (make it a one-liner)

icon mode

Button comment

Workspace I'm thinking this should look like: "CMake: Active Folder [${folder name}]\nClick to change"

Status I'm thinking this should look like: "CMake: ${status text}\nClick to select the current build variant" (In the future, maybe status and variant will be split out so we can put the variant in square brackets, but I think that's out of scope for this PR)

Kit I'm thinking this should look like: "CMake: Active Kit [${active kit}]\nClick to change the active kit"

CTest I don't think we need to show the button text in the tooltip. (make it a one-liner)

ctest in icon mode should have the annotation of how many test did pass.
=> second line is only visible if there are test results

i changed the labels (and tooltips) such that selected values (kit, folder, variant, targets) are in brackets and text is without bracket.

i didn't add Active XYZ to icon mode.
I don't think that it adds information because the second line of the tooltip already states what is meant.

I changed the tooltip of the workspace button to:
Click to select the active workspace and Click to select the active workspace (auto) to match the other tooltips.
i'm not sure if auto is enough or if we should use autoselect.
auto is visible if Cmake: Auto Select Active Folder is selected.

I think putting (auto) in the tooltip is confusing. I would rather that be removed.

src/status.ts

SchweizS · 2020-05-07T20:46:01Z

current state

src/status.ts

bobbrow

If you can just remove the (auto) text from the workspace folder tooltip, this will be ready to merge. Thanks again for working on this!

SchweizS · 2020-05-11T16:35:05Z

i didn't want to touch everything which is related to that feature, so i just commented out he
parts related to autoSelect in status.ts

bobbrow · 2020-05-11T16:56:14Z

That's fine. We will clean it up in a separate PR.

Stefan Schweizer and others added 6 commits April 21, 2020 11:38

removed unused warning button

852e287

statusbar update: now uses classes

b367244

added settings

3ecb255

refactoring: readability

01b1748

added color setting to ctest button

efd0f80

Merge branch 'develop' into statusbar-update_212_952_1189

f8a1243

bobbrow requested changes Apr 25, 2020

View reviewed changes

SchweizS added 2 commits April 25, 2020 15:34

addressed change requests

9153572

readded hideDebug/LaunchCommand functions to status

dbe2991

Zingam mentioned this pull request Apr 26, 2020

Launching Debug from the status bar doesn't pick up settings from Launch.json #1202

Closed

SchweizS requested a review from bobbrow April 28, 2020 05:01

bobbrow requested changes Apr 30, 2020

View reviewed changes

SchweizS added 5 commits April 30, 2020 07:33

rename: settings -> settingsName

a0c8cd0

removed getDefaultType

9523789

protected getType -> private _getType

2b5adb2

fixed button order

35c4380

typecheck on text length

de40098

bobbrow reviewed May 2, 2020

View reviewed changes

src/status.ts Show resolved Hide resolved

src/status.ts Outdated Show resolved Hide resolved

src/status.ts Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

SchweizS added 8 commits May 4, 2020 09:03

hide Status and targets in icon mode

798e21c

changed loca of Launch button

905431d

refactoring for readability

1f93be7

added default icon for CTest (beaker)

fix: ctest coloring now needs true to use colors

33b1a73

config update:

a93e509

- renamed type to visibility - added toplevel localization

added loca and default values to advanced settings

a3d6da2

added proper typecheck to kit length

33e8c8d

added target tooltip to Build, Launch and Debug

8cbef1a

added tooltip to BuildTargetName

SchweizS added 2 commits May 5, 2020 07:46

refactoring

226c4e6

- fixed access - reordered function to reflect access - renamed member to reflect access

improved compact mode of status button

91e2bec

SchweizS added 5 commits May 5, 2020 07:49

improved launch target selection

008cb13

improved compact mode of ctest button

9f6be09

added target to build, debug and launch buttons

d8cba03

clang-tidy

b6a7571

added missing type annotations

8209fe8

renamed ActiveFolderButton to WorkspaceButton

bobbrow added this to the 1.4.0 milestone May 5, 2020

bobbrow reviewed May 6, 2020

View reviewed changes

SchweizS mentioned this pull request May 6, 2020

Workspace Button doesn't always show up #1219

Closed

SchweizS added 2 commits May 6, 2020 08:13

loca update

cfc9ffb

tooltip update

4803f54

bobbrow requested changes May 7, 2020

View reviewed changes

finalizing for release

bae00e4

bobbrow reviewed May 8, 2020

View reviewed changes

src/status.ts Outdated Show resolved Hide resolved

active workspace -> active folder

f14d29d

bobbrow approved these changes May 11, 2020

View reviewed changes

SchweizS and others added 2 commits May 11, 2020 18:31

removed autoSelect note from active folder

6d4e4db

Merge branch 'develop' into statusbar-update_212_952_1189

9e2736b

bobbrow merged commit e21a430 into microsoft:develop May 11, 2020

bobbrow mentioned this pull request May 13, 2020

Add buildKitShort command, add shortName property to kits #1235

Closed

github-actions bot locked and limited conversation to collaborators Jan 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Statusbar update 212 952 1189 #1200

Statusbar update 212 952 1189 #1200

SchweizS commented Apr 23, 2020 •

edited

Loading

bobbrow left a comment

SchweizS commented Apr 25, 2020

bobbrow left a comment

bobbrow left a comment

SchweizS commented May 4, 2020

SchweizS commented May 5, 2020

macdems commented May 5, 2020

bobbrow commented May 5, 2020

bobbrow left a comment

bobbrow left a comment

bobbrow May 7, 2020

SchweizS May 7, 2020

bobbrow May 11, 2020

SchweizS commented May 7, 2020 •

edited

Loading

bobbrow left a comment

SchweizS commented May 11, 2020

bobbrow commented May 11, 2020

Button	comment
Workspace	I don't think we need to show the folder name in the tooltip because we don't truncate the text.
Status	I think the status should come before the command text. Can you switch the order?
Kit	I think the active kit should come before the command text. Can you switch the order?
Build	The text should start with "CMake:"
Build label	I don't think we need to show the selected build in the tooltip because we don't truncate the text.
Launch label	The text should start with "CMake:"
CTest	I don't think we need to show the button text in the tooltip. (make it a one-liner)

Button	comment
Workspace	I'm thinking this should look like: "CMake: Active Folder [${folder name}]\nClick to change"
Status	I'm thinking this should look like: "CMake: ${status text}\nClick to select the current build variant" (In the future, maybe status and variant will be split out so we can put the variant in square brackets, but I think that's out of scope for this PR)
Kit	I'm thinking this should look like: "CMake: Active Kit [${active kit}]\nClick to change the active kit"
CTest	I don't think we need to show the button text in the tooltip. (make it a one-liner)

Statusbar update 212 952 1189 #1200

Statusbar update 212 952 1189 #1200

Conversation

SchweizS commented Apr 23, 2020 • edited Loading

This change addresses item

This adds:

Changes:

This does not address:

Notes:

bobbrow left a comment

Choose a reason for hiding this comment

SchweizS commented Apr 25, 2020

bobbrow left a comment

Choose a reason for hiding this comment

bobbrow left a comment

Choose a reason for hiding this comment

SchweizS commented May 4, 2020

SchweizS commented May 5, 2020

macdems commented May 5, 2020

bobbrow commented May 5, 2020

bobbrow left a comment

Choose a reason for hiding this comment

bobbrow left a comment

Choose a reason for hiding this comment

bobbrow May 7, 2020

Choose a reason for hiding this comment

SchweizS May 7, 2020

Choose a reason for hiding this comment

bobbrow May 11, 2020

Choose a reason for hiding this comment

SchweizS commented May 7, 2020 • edited Loading

bobbrow left a comment

Choose a reason for hiding this comment

SchweizS commented May 11, 2020

bobbrow commented May 11, 2020

SchweizS commented Apr 23, 2020 •

edited

Loading

SchweizS commented May 7, 2020 •

edited

Loading