Skip to content

Conversation

@ninyawee
Copy link
Contributor

@ninyawee ninyawee commented Mar 5, 2025

  • Added new is-cursor command to detect Cursor editor
  • Created is-vscode-like command to generalize VSCode-like editor detection
  • Updated various commands to recognize Cursor alongside VSCode
  • Modified editor selection logic to include Cursor as a preferred editor

- Added new `is-cursor` command to detect Cursor editor
- Created `is-vscode-like` command to generalize VSCode-like editor detection
- Updated various commands to recognize Cursor alongside VSCode
- Modified editor selection logic to include Cursor as a preferred editor
@ninyawee
Copy link
Contributor Author

ninyawee commented Mar 5, 2025

im not sure if this has to be in beta first.

@balupton
Copy link
Member

balupton commented Mar 6, 2025

Cool, thank you. I'll review this properly after #281 is merged.

Copy link
Member

@balupton balupton left a comment

Choose a reason for hiding this comment

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

Todos before merging:

  • Remove is-vscode-like, change to is-vscode || is-cursor.
  • Do the minor changes.
  • Document why the numerous checks in is-cursor are necessary, if they indeed are.
  • Open an issue on Cursor's github requesting they change their TERM_PROGRAM to cursor instead of vscode, link that created issue in a comment in is-vscode

# Also check if we're in VSCode with Cursor-specific environment variables
if [[ ${TERM_PROGRAM-} == 'vscode' ]]; then
# Check for Cursor in environment variables
if env | grep -q "NAME=Cursor"; then
Copy link
Member

Choose a reason for hiding this comment

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

What is the point of these checks? Wouldn't it have been caught by the earlier ${NAME-} == "Cursor" check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But double conditioning is to make sure. Im not sure if name could be alter by others.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, let's remove this block then, and just do the earlier check. env will just reflect the current exported variables, which should be what is also shared by our current bash instance. For instance, if our bash instance changed NAME and it is exported, such a change will also happen for the called env. If NAME was no longer exported, same would occur for env. However, NAME could be a local variable, in addition to a parent's export variable, in this case, yes they could diverge, but is-cursor is running as a subshell command, so this should never be the case.


function __check {
is-ci || is-vscode
is-ci || is-vscode-like
Copy link
Member

Choose a reason for hiding this comment

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

I think we nuke is-vscode-like and just change this to is-vscode || is-cursor, as it could be possible that a vscode-like editor comes out hat doesn't have title support, and this seems to be the only use case of such a vscode-like detection.

# no terminal or gui preference, determine sensible defaults
if is-vscode; then
if is-cursor; then
editors+=(cursor)
Copy link
Member

Choose a reason for hiding this comment

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

missing # if running within Cursor, add Cursor as preference

gui='yes'
terminal='yes'
elif is-vscode; then
# if running within vscode, add vscode as first preference
Copy link
Member

Choose a reason for hiding this comment

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

change comment to remove first

done

if is-vscode; then
if is-vscode-like ; then
Copy link
Member

Choose a reason for hiding this comment

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

change to is-vscode || is-cursor

@@ -0,0 +1,53 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

let's nuke this command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants