Skip to content

Conversation

JunkuiZhang
Copy link
Contributor

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jul 9, 2025
@JunkuiZhang JunkuiZhang marked this pull request as draft July 9, 2025 13:32
@ConradIrwin ConradIrwin removed their assignment Aug 15, 2025
@localcc
Copy link
Contributor

localcc commented Aug 15, 2025

I'm not sure the way this PR accomplishes showing ctrl+shift+4 insted of ctrl+$ is the best way, why not just change the internal representation instead of wrapping it?

@JunkuiZhang
Copy link
Contributor Author

@localcc I did try the approach you mentioned, but both Mikayla and Conrad felt it would require too many changes and diverge too much from the current implementation, see #29144. That’s why we ultimately went with the solution in this PR.

@localcc
Copy link
Contributor

localcc commented Aug 19, 2025

we should probably have a discussion about this again i think, maybe there's a third way or a compromise that wasn't seen before

@JunkuiZhang
Copy link
Contributor Author

we should probably have a discussion about this again i think, maybe there's a third way or a compromise that wasn't seen before

Maybe you could chat with Conrad or Mikayla about it, I’ve already discussed different implementation options with them before. Given my English level, they’ll probably be able to give you clearer feedback. I’m not sure what you mean by the third way.

@JunkuiZhang
Copy link
Contributor Author

Closing this PR in favor of #36572, this one has way too many merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants