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

Escape $ in file paths #331

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Escape $ in file paths #331

merged 1 commit into from
Nov 10, 2023

Conversation

amsul
Copy link
Contributor

@amsul amsul commented Nov 9, 2023

The $ character in file paths gets treated as a variable by Sublime Text. And as a result, ESLint fails to lint TS files that are "not included" because the path doesn't match any that tsconfig has included.

For example, /some/long/file.$path.ts becomes /some/long/file..ts

With this change, the $ is escaped and therefore the full path stays in tact.

@kaste
Copy link
Contributor

kaste commented Nov 9, 2023

That's a good catch for an edge case. Introduced as a byproduct of #326. Notes for a future self:

Typically variables are injected using the information in self.context. Before #326 we had

    defaults = {
        ...
        '--stdin-filename': '${file}',
    }

And that $file is stored in the context. Following that pattern, we could (or should?) have defined maybe: '--stdin-filename': '${stdin-name}', and the set self.context["stdin_filename"] = self.get_stdin_filename() within cmd.

Alternatively: '--stdin-filename': '${file:fallback_filename}', and self.context["fallback_filename"] = "__buffer"...` because the variable injection/substitution system allows for fallbacks.

Let's think about this for a moment. Should we take the simple fix or try "to do it right" although it would be more code.?

The `$` character in file paths gets treated as a variable by Sublime Text. And as a result, ESLint fails to lint TS files that are "not included" because the path doesn't match any that tsconfig has included.

For example, `/some/long/file.$path.ts` becomes `/some/long/file..ts`

With this change, the `$` is escaped and therefore the full path stays in tact.
@kaste kaste merged commit 9771add into SublimeLinter:master Nov 10, 2023
2 checks passed
@kaste
Copy link
Contributor

kaste commented Nov 10, 2023

Let's start by merging this in. Thanks for finding the bug!

kaste added a commit that referenced this pull request Nov 11, 2023
If we use SublimeLinters standard way of injecting variables, we don't
need the watch out for `$` characters because SublineLinter will do.
This is what we did before #326 as well.

This is a follow-up of #331.
kaste added a commit that referenced this pull request Nov 11, 2023
If we use SublimeLinters standard way of injecting variables, we don't
need to watch out for `$` characters because SublineLinter will do.
This is what we did before #326 as well.

This is a follow-up of #331.
kaste added a commit that referenced this pull request Nov 11, 2023
If we use SublimeLinter's standard way of injecting variables, we don't
need to watch out for `$` characters because SublineLinter will do.
This is what we did before #326 as well.

This is a follow-up of #331.
@amsul
Copy link
Contributor Author

amsul commented Nov 12, 2023

You’re welcome! Discovered as a byproduct of using Remix with dynamic route paths in the file name.

Would be great to do it the right way but python is certainly not my forte. Let me know if I can help in another way 👍

@kaste
Copy link
Contributor

kaste commented Nov 12, 2023

There is a follow-up #332 where I did it "the-right-way".

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

Successfully merging this pull request may close these issues.

None yet

2 participants