-
Notifications
You must be signed in to change notification settings - Fork 511
Double clicking variable selects variable AND $ prefix - this is not desirable #3378
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
Comments
Is this sufficient for you? Since it's only a supplied default, the behavior is easily overridden.
This was a small change I made based on feedback gathered during a VS Code PowerShell extension meetup. See #3359 It seemed like most users would prefer the ISE-like behavior were - and $ are not word separators. What if we put |
I've got no problem leaving |
Just playing around with this, the included change makes the VS Code experience consistent with PowerShell itself. That is, if you put the cursor on |
I'm definitely not one of those happy with this change. It messes with Ctrl+Backspace, so erasing and retyping a variable name means also having to retype the $, and erasing a parameter also erases the trailing -. It messes with quick refactoring, so Ctrl+F2 to change the name of a variable means, again, retyping the $, even though it makes no sense that I'd want to change that part since all variables must be decorated with a $ (not to mention, it means splats won't be updated in a quick Ctrl+F2 refactor, which can actually introduce bugs/break scripts) Not to mention, it makes refactoring multiple functions in the same module to change the noun or prefix a bigger pain, because now I either have to do it for every verb variant or write a regex find/replace for it. In any event, these are behaviors that I've come to expect from the editor in the several years I've been using Code to write my PowerShell, and this change has already caused a significant amount of aggravation in just the very short time since it's been implemented, to the point I had to dig in to the settings to figure out why it wasn't working as expected, and even went to the hassle of searching the commit history for this repo just to verify I wasn't going mad and this had actually changed. This is a breaking change to existing user workflows. I don't see the old behavior as confusing in the slightest, and personally feel that existing users who have already come to expect certain behavior should take precedence over new users who can learn the existing behavior more easily than we can re-learn it. Four plus years of muscle memory don't fade overnight. Anyway, I know there's a workaround and will be using it, but I really think this was a poorly thought out decision. |
Per the changelog (which should show up automatically when the extension updates) you can revert with: "[powershell]": {
"editor.wordSeparators": "`~!@#$%^&*()-=+[{]}\\|;:'\",.<>/?"
} |
💯 Amen brother! |
I don't think its that simple. I've hated it for years that you can't double click to select the whole variable with the If triple-clicking would have selected the whole word instead of the line, it would be a good compromise. I don't know if that is possible in VS Code. So what scenario should be optimized here? Refactoring, Copying, transforming to splatting or string interpolation? Is there a way to paste with special options like Office does to auto-refactor it? That would be a cool feature. |
The way I see it, with copying, you have three options:
Since PowerShell, as far as I can see (at least of the languages installed on my system), is the only language provider that now modifies the default list of word separation characters, that means even other languages that use the $ variable decorator don't mess with word separator list. And I will say, for me personally, the minor issues caused by the default word separator list from VSCode are easily overcome with small adjustments to expectations to match, while the issues caused by the modified list means the built in refactoring tools become noticeably less useful, forcing manually using find/replace in a lot of scenarios that a simple Ctrl+F2 should have been able to handle. And, again, this is a breaking change to existing user workflows to, as far as I can tell, reduce the learning curve, though it seems to me that it doesn't do much on that end, especially compared to the price of making experienced users' tools less functional. As for the changelog showing up automatically...I haven't seen one in a long while myself, didn't even realize it had updated until I tracked this issue down to it, and just to verify, I launched it on a system I've not coded on for the last few weeks and all I got was a pop-up notification that went away after ten seconds, something that I'm likely not to notice or even bother looking at when I'm in the middle of getting something done. You can say that's my own fault, but I don't think it's an unreasonable expectation that existing workflows won't break because of a change in default behavior as opposed to adding support for changing our own workflows. Since this update also marks the third time I've had to chase down why variable properties are no longer colored differently than the variables themselves because the name they've been given for TextMate rules has changed, again, I'm genuinely confused by the direction the team running this project have chosen. I don't know about you, but having my tools break in updates costing me an hour or two hunting down what changed and how to fix it isn't exactly a stable, production ready experience. This attitude of "move fast and break things" doesn't belong in professional tooling. Anyway, I've said my piece, I've fixed my tools and can go about getting my work done and can put this behind me (at least until the team running this project decides to break things again), so I'll be off. Please consider changes like this more carefully in the future. And please consider reversing this one as I'm obviously not the only existing user frustrated by this change, especially since anyone who likes this new behavior could have easily set up their own install to do it had they wanted to at any point in the last five years. |
I'm kind of puzzled about the trend to keep changing defaults to more closely mimic the ISE when there is a palette command "PowerShell: Enable ISE Mode" that was added specifically to make it easy to change a number of settings to values that more closely mimic the ISE. "PowerShell: Disable ISE Mode" returns those values to the extension defaults, but if those defaults are already ISE-like and foreign to non-PowerShell VSCode users, what is the point? The main reason I started using VSCode was because I was sick of having different editors for different languages that behaved in unique and cognitively conflicting ways. With VSCode I didn't have to mentally switch back and forth between the text selection behaviours of two DE's and it still did sensible language-specific things like code highlighting and IntelliSense. Common debug shortcut keys and predictable keyboard-based navigation was a huge reason for that. Obviously, having the option to change this particular setting back is welcome in this case, as other changes ISE-ward have not provided configuration options to restore the old behaviour(#2523). In general I question the wisdom of this overall approach. I completely understand that people either like the way ISE works but want the more advanced features of VSCode, or simply want an easier transition to a more modern tool. I think a separate extension for a PowerShell ISE Mode sounds very sensible and something similar has been done for plenty of other languages. But again, the "Enable ISE Mode" command already exists, supposedly for this very purpose. More relevant to the discussion at hand -- take this code, a scenario I run into quite often: $BaseName = 'CSL'
$Seperator = '-'
$Date = Get-Date -Format 'yyyyMMdd'
$Extension = '.dx1'
$NewReportFileName = "$BaseName$Seperator$Date$Extension" The new default value makes selecting a portion of that string unwieldy. As a bonus, if I want to correct the spelling of that $Seperator variable name this won't work either, as mentioned in an earlier comment. Previously I could just click somewhere in the middle of the name on the second line (notice that it visually highlights the variable as expected), then press + to highlight all instances of that variable name. With the current default value, the string on the last line is not updated with the rest. Furthermore, the exact same problems occur if the last line uses a literal '-' character: $NewReportFileName = "$BaseName-$Date$Extension" tl;dr: |
As far as I can tell, this has nothing to do with ISE. ISE does not include So why this change? Because without further documentation, to alot of users this could appear to be a bug or quirk. Check the following issues about missing I think including Ultimately, this issue is a set of trade-offs. Maybe one day more advanced solutions can be found so we can satisfy all use cases like I mentioned before. Until then, I would suggest using the established behavior from ISE for both |
@andschwa sited ISE-like behaviour as the goal for this change, so that was the basis of my comment. If this seems like a bug for new users, then it would likely do so for other languages as well and would perhaps make more sense to address it in vscode itself, rather than making this an exception for only PowerShell. Again, consistency is a valuable feature of vscode that helps those who work in multiple languages to be more efficient and that means being judicious in choosing seemingly trivial things such as default settings. That said, at least the default can be changed back. |
What is considered a word is different per language. VS Code did address the issue by allowing
Consistency is a relative term. Consistent to what? I would say it is very inconsistent that double selecting a variable in PowerShell does not include Now, we learn through examples in this issue that not including As for consistency of default values between builds, that is never guaranteed by VSCode itself. New defaults are chosen all the time in VS Code releases that change behaviors. They just have the benefit of alot of people testing insider builds to catch feedback early on and even then, they may get negative feedback on release. That is expected for this editor. I think we need to remember that alot of people have different ways of using software and it often conflicts with each other. The comments on this issue do not reflect the general community because its just 3 users reporting an issue out of the 3 issues that were opened to include I do hope some exploration is done to make it possible to solve the problem for all users and workflows but that I don't think will happen anytime soon given the complexity. |
The consistency I'm talking about is basically this: High level languages don't differ much in how they are constructed semantically. Even in what they do and do not consider to be a "word". Therefor there should be few exceptions, if any, with how one is to navigate text of any high level coding language. It really is just text. PowerShell in particular isn't unique in terms of the semantic constructions that make up the language. The use of a special character to identify a variable is pretty common in fact. I've looked at a few languages with this general approach to variables that I could think of off the top of my head and they all behave the same way in vscode; the special character is not treated as part of the variable name. In fact, semantically I would argue that the If a higher concept such as "this text represents a variable and should be grouped like a word" is thought to be valuable, I think the best approach would be to design it into vscode and implemented as an optional feature for any language, not just in the PowerShell extension. This could simplify things like code auto-formatting for example. There is already code (in I think the code highlighting) that does a great job of auto-highlighting complex variable tokens such as this, but I don't know how it relates, if at all: I have particular opinions about UX design with respect to code editors, but ultimately if features or specific behaviour are worth implementing, they should be implemented at the appropriate place to provide better code efficiency and UX consistency. That's all I'm really advocating here. If a lot more rambling that I was hoping. |
@dragonwolf83 Changing three years of EXISTING behavior was probably not a good idea IMO. That is one issue I have with certain OSS projects. Folks seem a little cavalier with making changes to well established behavior. At the very least, the change in behavior should have been made "opt-in". |
That is exactly how this works. Nothing is hard-coded here. The VS Code feature is a global setting called The setting can be customized by user, workspace, or language. All of what you are asking for or suggesting here is already done. All the PowerShell Extension team did is default to what appeared to make sense based on general feedback they got. You can undo it by changing the setting yourself.
It behaves the same across languages within VS Code because To understand proper behavior, you would have to compare how it works with other IDEs and ideally native IDEs for a specific language. I just compared 3 different ones: Padre for Perl, PowerShell Studio which I believe predates ISE, and PyCharm for Python. Both Padre and PowerShell Studio include
@rkeithhill I would agree we need far more data and polling in the PS community about sensible defaults and behaviors. The fact is, we don't have hardly any representation in this issue to really know what the user base wants here. We should appreciate how hard it is to accommodate conflicting feedback from users that wanted We should also appreciate that we are not always going to know what is the best solution until we have enough users trying it out. I certainly never thought of the issues you cited, but you raised valid use cases. What probably needs to happen next time is make sure a clear issue is open for feedback so that people can discuss. Next problem is how to get more discussion happening in this repo so that more than just 3-4 users are dictating the law for everyone. |
I found this issue after noticing the select behavior of the PowerShell extension changed (to the worse in my perception), but I'm really thankful there is a workaround that brings back the old behavior. So if I may suggest something for the future: changing default behavior that has been consistently the same for years is something that needs more consideration, and maybe an opt-in possibility instead of an opt-out would be friendlier on the regular users. |
Perhaps we move the |
From what I tested ISE doesn't include It just needs to default with |
Sounds good, thanks all for the feedback. I'll make the change to put |
I don't think we should keep the hyphen exclusion either. We've been focusing on the dollar sign, but I think most of the arguments against that change apply to both. Certainly PowerShell cmdlets are explicitly formed as Verb-Noun pairs and as such there is a lot of value in handling them as separate words. For my own workflow, I regularly replace just the "verb" portion of the cmdlet. For example, I'll duplicate a line and change the verb on the new line to perform a complimentary operation with similar arguments. As for arguments marked by hyphens, PowerShell isn't unique here either and none of those other languages make this particular exception. Again, I advocate for using the global default for word separators, both for the sake of |
System Details
System Details Output
I had to run this from pwsh. PSIC just hangs when I try to run this command. But PSES is:
Issue Description
Double-clicking a variable has changed to now selecting the
$
prefix. This is a problem when pasting over the variable when used in splatting e.g.@iwrArgs
becomes@$iwrArgs
. Ditto with string interpolation"${smtpServer}"
becomes"${$smtpServer}"
after pasting. This is highly undesirable IMO.Expected Behaviour
Double-click select of a variable should not include the
$
in the selection.Actual Behaviour
Double-click select of a variable does include the
$
in the selection.The text was updated successfully, but these errors were encountered: