Skip to content
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

Extend support for simple transformation flags to sed-style replacements #20

Conversation

savetheclocktower
Copy link

This one will be hard to explain, but let's start with this StackOverflow answer as an example of something that VSCode supports.

${1} -> ${1/(.*)/${1:/upcase} ${1:/downcase}/}

This might look ridiculous, but if it's a snippet body, you should be able to invoke the snippet, type Lorem Ipsum, and get

Lorem Ipsum -> LOREM IPSUM lorem ipsum

The /upcase//downcase-style flags are easier to use than figuring out the fancier syntax for case transformation, and flags like /camelcase and /pascalcase are even handier; if we'd used those two instead, we'd have gotten

Lorem Ipsum -> loremIpsum LoremIpsum

We already supported these flags as ways of transforming entire variables — ${CLIPBOARD:/upcase}, et cetera — but I had assumed we supported them for tab stops also. We don't. VSCode and TextMate both support this usage, so we should as well.

I added some tests to demonstrate what this does, and all the existing tests pass.

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

To be honest, this is a bit much for me to catch up on before 1.116.0 would go out (tentatively the 15th in two days as per usual by loose tradition).

So, I'm inclined to "rubber-stamp approve" this so it can be added in time for 1.116 if you believe it's ready.

If I can figure out how to manually test this, I will attempt to do so with enough time to help review this more thoroughly before 1.116 would happen.

@savetheclocktower savetheclocktower force-pushed the support-transform-flags-on-sed-replacements branch from e779a47 to 4235978 Compare April 14, 2024 02:30
@savetheclocktower savetheclocktower merged commit f10649f into pulsar-edit:master Apr 14, 2024
3 checks passed
@DeeDeeG
Copy link
Member

DeeDeeG commented Apr 14, 2024

Some post-merge "review-ish" discussion on Discord, re-posted here for relevance and findability, for posterity, serves as a slightly higher standard of review than just "rubber stamp, lol."


DeeDeeG:

@savetheclocktower is this right? "lorem" isn't being transformed like "ipsum dolor" is? (Or is it maybe just a peculiarity of the test output, but working properly in-app?)

expanding: v_simple_upcase
TEXT: lorem Ipsum Dolor lorem IPSUM DOLOR
.expanding: v_simple_pascalcase
TEXT: lorem Ipsum Dolor lorem IpsumDolor
.expanding: v_simple_snakecase
TEXT: lorem Ipsum Dolor lorem ipsum_dolor

https://github.com/pulsar-edit/snippets/actions/runs/8677860906/job/23794070630#step:6:61

savetheclocktower:

yeah, those test snippets are all designed to test the idea that you can apply some transformation flags to a particular reference in a sed-style replacement: https://github.com/pulsar-edit/snippets/blob/master/spec/snippets-spec.js#L352-L363

so to do that, i have the test snippets transform the part after the first word

DeeDeeG:

Oh, I see. Whew. 😅
https://github.com/pulsar-edit/snippets/blob/v1.8.0/spec/snippets-spec.js#L1115-L1119

savetheclocktower:

heh, i certainly didn't mean to leave those console.log statements in there in the first place


NOTE: Now that I'm posting this here, hmm, maybe we can remove the console.log()s now, I dunno if it's worth a 1.8.1 version, but probably should be done at some point to avoid spam in the Dev Tools console, or for users that launch Pulsar from the CLI.

@savetheclocktower
Copy link
Author

Now that I'm posting this here, hmm, maybe we can remove the console.log()s now, I dunno if it's worth a 1.8.1 version, but probably should be done at some point to avoid spam in the Dev Tools console, or for users that launch Pulsar from the CLI.

Good news is that it only affects the specs, but I'll make the change to my local copy now so that it's present the next time I have to touch the repo.

@savetheclocktower savetheclocktower deleted the support-transform-flags-on-sed-replacements branch April 14, 2024 04:42
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.

2 participants