Skip to content

Escape '$' when launching Scalpel #11

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

Merged
merged 1 commit into from
Jun 12, 2020
Merged

Conversation

kengkiat
Copy link
Contributor

Fixes #10

I've decided to only escape $ as I can't think of any other special characters where this is useful. However, this can easily expanded to include other characters.

@kengkiat kengkiat marked this pull request as ready for review June 10, 2020 02:19
@wincent
Copy link
Owner

wincent commented Jun 10, 2020

With something like this we have a few ways to do it:

  • Change it straight up like you have here.
  • Change it but make it optional (and if optional, decide whether to have the option off or on by default).
  • Make it configurable (eg. let the user specify a list of characters to escape... let g:ScalpelAutoEscape='$').
  • Make it "smart" so that no configuration is necessary (eg. look for characters in the current filetype's 'iskeyword' and see if any of them should be escaped).

I almost never want to do the first option, in case there's some use case I haven't thought about; I don't want to risk breaking somebody's usage scenario.

I think my preference here is basically in the order of the list above (from least to most preferred):

  1. Change it for everybody.
  2. Make it optional (off by default).
  3. Make it optional (on by default).
  4. Make it configurable.
  5. Make it "smart".

The only risk is that sometimes "smart" things end up being dumb. 😂 Any thoughts on this?

@wincent
Copy link
Owner

wincent commented Jun 12, 2020

Ok, thinking about it, it seems that we can just blanket-escape anything that might have special meaning for /\v. I don't think there is risk of this breaking anybody's workflow, and I don't think we need to try and be "smart" and vary the behavior based on what's in 'iskeyword'.

Thanks for the PR!

@wincent wincent merged commit 1a796c4 into wincent:master Jun 12, 2020
@kengkiat
Copy link
Contributor Author

Sorry for the late response. I think it make sense to make it configurable (but with sensible defaults). Will probably submit another PR when I have the time.

@kengkiat
Copy link
Contributor Author

Nvm... I see that it's done :)

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.

Escape special characters when starting Scalpel
2 participants