Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Adding '$' character for highlight-selected #412

Merged
merged 1 commit into from
Jan 29, 2021

Conversation

JesseSteele
Copy link
Contributor

@JesseSteele JesseSteele commented Jan 24, 2021

Requirements

I have tested this by making a git clone into '/usr/lib/atom/node_modules' and it behaved as intended, with no bugs or warnings.

Description of the Change

Changes the default value of of "Non Word Characters" in language-php to match the solution in the thread here:

https://discuss.atom.io/t/double-click-highlights-whole-of-variable-in-php/18625

The thread above suggests (from 2015):

$/()"’:,.;<>~!@#%^&*|+=[]{}`?-

However, adding a $ character to the current settings would result in the requested change here, being:

$/\()"':,.;<>~!@#%^&*|+=[]{}`?-

...This is the new value used in this pull request (2021).

Alternate Designs

NA: There were no alternatives found because this worked on the first try. Since it was also least invasive, trials went no further.

Benefits

Double-clicking on a PHP variable is not picked up by highlight-selected or its minimap-highlight-selected variant. This resolves that issue so double-clicking on a PHP variable not only highlights the name of the single variable, but highlight-selected picks up other occurrences.

Possible Drawbacks

The main drawback would be if the problem lies within the highlight-selected package on a deeper coding level.

However, philosophically, this should probably be the default value anyway, as per the Atom discussion. If there is a deeper problem with the highlight-selected package, then this bug here helped identify that problem, but this default value should probably be changed anyway.

I'm not an expert in the full code; just doing the leg work rather than adding to other people's to-do lists. I suppose a better way might exist. If so, by all means. But, the problem wasn't solved and this solves it by the official Atom discussion. Cheers!

Applicable Issues

This does relate to the higlight-selected package.

For discussion: Without this fix, the same text still highlighted on double-click. It was highlight-selected that failed to pick it up. So, it seems this setting only affected another package (highlight-selected), not the native select capability itself. That suggests that other packages may depend on this setting more than native Atom packages do. Not knowing the full code, I don't know, but it seems strange. Still, this fixed the problem by the official discussion I read.

Per this thread at Atom: https://discuss.atom.io/t/double-click-highlights-whole-of-variable-in-php/18625

Double-cliccking a variable is not recognized by highlight-selcted by the original default setting for Non-word characters. Adding a '$' character to the beginning fixes this problem so highlight-selected pics it up.

If there is another way to solve this problem so double-clicked variables highlight, by all means, do that instead.
@KapitanOczywisty
Copy link
Contributor

KapitanOczywisty commented Jan 24, 2021

In my opinion, as default double-click should highlight variables as a whole - with $. It's currently bugged, but I hope will be fixed #92 (comment)

@keevan
Copy link

keevan commented Oct 1, 2021

So it seems like this issue has been flip flopping for a while now.
#92 (comment)

I personally use vim (vim-mode-plus) and find this incredibly frustrating whenever the language gets automatically updated. Where before I would yw to copy the $variable, I now have to do ye or some other variation. Bit worse when I'm in the middle of the word (can't just do yiw like I used to.

I say, whichever way it flips, set it, and stop changing it, and let people who prefer one or the other configure it. If it becomes a big enough deal, add a config checkbox for it specifically. I just hope that my non-default without the $ will stay even if it becomes the default again, and inevitably flips again...

/\()"':,.;<>~!@#%^&*|+=[]{}`?-

@KapitanOczywisty
Copy link
Contributor

@keevan VScode has also vim extension and works way better than atom, just saying...

@keevan
Copy link

keevan commented Oct 2, 2021

@keevan VScode has also vim extension and works way better than atom, just saying...

@KapitanOczywisty I respect your opinion. Do you know if VSCode's vim extension has something similar to VMP's occurrence selector (https://github.com/t9md/atom-vim-mode-plus/wiki/OccurrenceModifier#normal-operation-vs-occurrence-modified-operation), being able to visual mark words (which does a select on the current smart-word, and you can use caf to change the occurence of it in a function after visually confirming it's what you wanted to change and/or jump through them), as well as atom-narrow (https://github.com/t9md/atom-narrow). I use these 2-3 extensively so without them I'm finding it hard to switch to anything else.

Related discussions:
t9md/atom-vim-mode-plus#1132
t9md/atom-vim-mode-plus#1063

That said, getting off topic from this issue. So I'll leave it at that.

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

Successfully merging this pull request may close these issues.

4 participants