johnpapa / vscode-cloak

Cloak allows you to hide/show environment keys, to avoid accidentally sharing them with everyone who sees your screen.
MIT License
105 stars 7 forks source link

feat: support JSON files [ON HOLD] #19

Closed manekinekko closed 2 years ago

manekinekko commented 4 years ago

This is a quick PR that adds support for JSON sources. For now, we just process values in double-quotes and numbers (in all JSON files).

ezgif com-video-to-gif (2)

I will try to figure out a way to allow users to select certain files only. But this might require a lot more changes.

Closes #18

johnpapa commented 4 years ago

Won't this affect all JSON files? Like package.json, settings.json, etc?

manekinekko commented 4 years ago

Yes. It will.

I need to figure out a way to allow users to select certain files only. But this might require a lot more changes.

johnpapa commented 4 years ago

Understood. Can’t merge unless we do figure that out as it hides too much :)

I hope we can find a way

manekinekko commented 4 years ago

I am onto something right now. Will update the PR when I have some results ;)

johnpapa commented 4 years ago

Thanks for the PR. I am reviewing it now. I pushed some changes to add the new setting.

A few ideas I am pondering:

  1. currently this will only work for the active document. You could have multiple documents open, so it will need to be adjusted for that.

  2. I think an array in the settings simplifies the code. I'll make that change

  3. are they globs or regex? I think we should decide. Ideally it's just a file pattern, regardless of folder location. So a regex may be best. I want to avoid exact file names, as that would be painful to enter several.

  4. there should be sensible defaults, like .env* and local.settings.json

  5. we need tests.

  6. the code will need some refactoring to fit the patterns I used. I'll do that, no worries.

  7. the code is currently in the toggle function. this means if you use the hide or show commands, it wont work. So once this is proven out, it would have to be moved and refactored for that.

again - thanks!

I'm working on some of this now ... give me a little and I'll circle back

johnpapa commented 4 years ago

So I made progress on this and I could refactor to address all of the above. However one problem still exists, and it's the original problem. Even if you detect the files that are open and match them to an array, the thing that actually does the hiding and showing are textmaterules. That is where the problem is. Those rules only apply to what textmate knows ... there is now way to make them apply to a file or pattern.

Also, the pattern is now applying to all json values (which we could adjust).

See below ....

2020-04-04_18-55-35 (1)

So this leaves us where we started ... needing to find a way to apply those rules to a set of files OR find a way to cloak that doesnt use the textmate rules

johnpapa commented 4 years ago

For my own notes, here is the textmate rule set that would be needed to apply to json and env

  "cloak.environmentKeys": "string.quoted.double.json,string.quoted.single.json,constant.numeric.json,constant.language.json,string.quoted.double.env,string.quoted.single.env,constant.numeric.env,constant.language.env,source.env",

note that there is no known way (to me) to limit that to a file pattern

johnpapa commented 4 years ago

And here is the full textmate rule doc explaning how filetypes can work. but this is not implemented in vs code, AFAIK.

https://macromates.com/manual/en/language_grammars

johnpapa commented 4 years ago

Another idea ... I can define a new grammar that covers local.settings.json. If I can figure that out, we may be able to apply the textmaterules to that. If that works, great ... but then the files are hardcoded into the extension. It can;t be a setting, AFAIK.

I'm taking notes here as I go.

manekinekko commented 4 years ago

These are good notes.

Here my answer to some of them...

are they globs or regex? I think we should decide. Ideally it's just a file pattern, regardless of folder location. So a regex may be best. I want to avoid exact file names, as that would be painful to enter several.

I initially wanted them to be globs. But I ended up using JS RegExp because we only need to match file names. However, it'd be useful to support stars * in the file names, eg. .env* or *.json. Note that *.json is not a valid JS RegExp.

there should be sensible defaults, like .env* and local.settings.json

Yes.

we need tests.

Yes.

Here is another thing I figured out while working on this issue: I have considered writing a custom Textmate grammar for the JSON values we want to process. Then, I found out that you can just append the source type to the scope, ie. .json for JSON: string.quoted.double.json. A scope of string.quoted.double would target all matched scopes, regardless of the source type. This would have been ideally combined with the filtering option we added. However, Textmate scopes don't apply to some sources, eg. .txt and probably others.

Then, I was wondering if we could simply move away from Textmate's scope and have our own "way" of processing values, but this would require a lot of changes, and I needed your opinion on this before moving on.

johnpapa commented 4 years ago

A textmate rule would allow customizing our own scope., But I could not figure out any way to make this apply to specific file names. You can check out the textmate grammar docs for this. VS Code allows us to create a grammar. I was able to create a grammar and have it label specific scopes thigns like cloak.secret. But, I could not get it to work for a specific file name.

The textmate grammar does support file types, which is an array of file extensions. But that doesn't help in this case either.

Of course cloak could take another direction ... if you want ot do that, feel free to discuss in a new issue. I'll keep this PR open for now in case on of us has an idea.