Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom formatting of display name #359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tylerlaprade
Copy link

I always wish I could show the mode in the status bar in uppercase so that I notice it more. This is how Vim does it, I believe. Rather than making a simple boolean, I made it an enum so other display modes can be added in the future. This would be helpful for someone who has a lot of custom modes and wants to change how they're displayed.

This is my first time working on a VSCode extension, so feedback is very welcome. I wasn't sure how to build. I ran npm i, but it change the yarn file and added far more to package-lock.json than I expected, so I reverted it. I also didn't touch any validation of the setting (although I did add a default to the switch case to handle unrecognized values).

@bezbac
Copy link

bezbac commented Jan 30, 2025

Hi! Just FYI, it's already possible to customise this (albeit a little hacky) using a library such as https://github.com/be5invis/vscode-custom-css.

Screenshots:
Screenshot 2025-01-30 at 09 53 50
Screenshot 2025-01-30 at 09 53 37

Here is the css styles I use myself:

#gregoire\.dance {
  order: -1;
  background-color: #959da5;
  color: black;
  font-weight: 700;
  text-transform: uppercase;
  border-right: 1px solid rgb(19, 19, 19);
  width: 10ch;
  display: flex;
  justify-content: center;

  .codicon {
    display: none;
  }

  &:has(*[aria-label="zap  normal, Dance - Set mode"]) {
    background-color: #6a737d;
  }

  &:has(*[aria-label="zap  insert, Dance - Set mode"]) {
    background-color: #85e89d;
  }

  &:has(*[aria-label="zap  select, Dance - Set mode"]) {
    background-color: #9ecbff;
  }

  &:has(*[aria-label="zap  magit, Dance - Set mode"]) {
    background-color: #d76496;
  }

  &:has(*[aria-label="zap  <no active mode>, Dance - Set mode"]) {
    background-color: #6a737d;
  }

  &:has(*[aria-label="zap  <no active mode>, Dance - Set mode"]) a {
    display: none;
  }

  &:has(*[aria-label="zap  <no active mode>, Dance - Set mode"]):before {
    content: "None";
  }
}

You can find the full reference in my dotfiles.

@tylerlaprade
Copy link
Author

Thanks for the suggestion - I am not interested in tweaking my CSS, though, since that's going to require more ongoing maintenance as things break due to future changes.

Copy link
Owner

@71 71 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! One thing to keep in mind for this PR: #338 adds "namespaced modes" like helix/normal. We should take this into account when changing the formatting of the mode IMO (e.g. would be nice to show/hide the prefix helix/).

package.json Outdated
@@ -930,6 +930,21 @@
"Selections are anchored to characters, like Kakoune; that is, they are positioned *on* characters, and therefore cannot be empty. Additionally, one-character selections will behave as if they were non-directional, like Kakoune."
],
"markdownDeprecationMessage": "Built-in modes are deprecated. Use `#dance.modes#` instead."
},
"dance.activeModeDisplayStyle": {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this name, but I wonder if we should try to match the CSS property text-transform, since it accomplishes something very similar. Something like activeModeDisplayTextTransform?

package.json Outdated
"as-is",
"uppercase",
"lowercase"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change

package.json Outdated

],
"default": "as-is",
"description": "Controls how the active mode is formmated in the status bar.",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "Controls how the active mode is formmated in the status bar.",
"description": "Controls how the active mode is formatted in the status bar.",

package.json Outdated
"default": "as-is",
"description": "Controls how the active mode is formmated in the status bar.",
"enumDescriptions": [
"Display the name with its original formmatting.",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Display the name with its original formmatting.",
"Display the name with its original formatting.",

@akdor1154
Copy link

Tangential, but i tackle the "oops i forgot which mode im in" by customizing the cursor - are the defaults reasonable here do you think?

@tylerlaprade
Copy link
Author

Thanks for the PR! One thing to keep in mind for this PR: #338 adds "namespaced modes" like helix/normal. We should take this into account when changing the formatting of the mode IMO (e.g. would be nice to show/hide the prefix helix/).

Very interesting! I'm not sure if that should be a separate setting, since the menu would start getting very long if we start getting into exponential combinations of possible settings. There'd be 5 menu entries at the moment (2x2 plus "as-is"), at a minimum.

I addressed all the other feedback, although I can't figure out how to run tests locally.

Copy link
Owner

@71 71 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tylerlaprade Thanks for the changes! You should be able to run the tests from VS Code, in "Run and Debug > Run all tests". I recommend launching the extension with "Launch extension" instead though.

I'm not sure what you intended, though: the configuration is documented as being set per-mode, but the code you added reads a global property instead.

@tylerlaprade
Copy link
Author

I'm not sure what you intended, though: the configuration is documented as being set per-mode, but the code you added reads a global property instead.

@71, that's right, I intended it to be global. Open to suggestions or edits if the documentation isn't clear!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants