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

Use Base64 to pass text through compiled erb templates #143

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

martinemde
Copy link

This is a proposed way to potentially reduce confusion in the error highlighter in rails.

The problem is partly that the highlighter looks for the source erb expressions in the compiled template. I addressed this problem in rails/rails#53657 by making the error highlighter more tolerant of this situation. It also occurs in rails easily, for example searching for a space between erb tags. Now the highlighter succeeds more often.

The other big problem is that multi-line blocks of code get double the newlines added to the compiled template. This causes the highlighter to fail to find the correct line in the first place and, until rails/rails#53696 merges, this can cause the highlighter to crash completely.

I think the main reason for this is the duplication of the code in the template. For this I propose encoding the string version of the template with base64 (urlsafe doesn't add a newline that I have to trim). This keeps the string version of the code from visibly existing in the template.

Hopefully someone with more experience with better-html can validate this approach.

This is a proposed way to potentially reduce confusion in the error
highlighter in rails. The problem is partly that the highlighter looks
for the source erb expressions in the compiled template. The other big
problem is that multi-line blocks of code get double the newlines
added to the compiled source, causing the highlighter to fail to find
the correct line. This attempt to reduce the likelihood of those
issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant