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

proposals for better quote-handling #49

Closed
shaunlebron opened this issue Nov 18, 2015 · 7 comments
Closed

proposals for better quote-handling #49

shaunlebron opened this issue Nov 18, 2015 · 7 comments

Comments

@shaunlebron
Copy link
Collaborator

I waited too long to write these down and it's not fresh on my mind anymore, but here are some suggestions from clojure conj folks:

  • @SVMBrown suggests a new two-step process. Upon loading a file, fix unclosed quotes in all comments. And then enforce all subsequent additions of quotes in comments are balanced or escaped.
  • @jteneycke suggests explicitly treating the jsdoc comments as special and untouchable (since escaping quotes would result in invalid jsdocs), while correcting all other comments.

But I'm now recalling a goal which seems to prevent these from working: we want to preserve reversibility. Imagine commenting and uncommenting a line of code. In my mind, the resulting line shouldn't be different (i.e. 1 and 4 below should be the same):

  1. We have a line with a quote:

    "foo
  2. We comment it out:

    ; "foo
  3. Quote fixer escapes it

    ; \"foo
  4. Uncomment

    \"foo

We also talked about how the source of the problem is that quotes are non-directional, and how their meaning is dependent on the number of quotes before it, allowing strings to be turned inside out when parity changes. svmbrown proposed that we imagine how chaotic this would be if parens were non-directional.

@SVMBrown
Copy link

Upon thinking this over, I think the problem runs deeper than comments. The insertion of an unmatched quote can break any file with pathological strings in it, as it will toggle all subsequent quotes. Perhaps parinfer could be suspended until a matching quote is provided.

perhaps it could ensure that the number of valid quotes is even before parsing. This may be limiting, however, and depends on ease of detecting validity of quotes.

(by valid, I mean quotes which serve as string endpoints i.e. not commented or escaped)

@shaunlebron
Copy link
Collaborator Author

@SVMBrown processing is in fact suspended when quotes are imbalanced (see string test cases). That does not fix this problem.

for example, valid number of quotes (2):

")))" ; "

insert a quote in the beginning, and there are still a valid number of quotes (4):

"")))" ; "

but the processing will erase the parens, now that they're considered code:

""" ; "

@shaunlebron
Copy link
Collaborator Author

another problem related to this came up: https://github.com/shaunlebron/parinfer/issues/56

@snoe
Copy link

snoe commented Nov 24, 2015

I bet I'm missing a fail case here (are there examples of quotes messing up without comments?) but would something like this work?

  • remove comments before transforming
  • transform but if a comment is created from the transform, suspend/mark as an invalid transform
  • put comments back

@shaunlebron
Copy link
Collaborator Author

After mulling over this for a week with some state machines and some discussions with @snoe on slack, here's my current plan.

Create a trapdoor to disable parinfer once odd number of quotes are found in a comment, and force the user to re-enable parinfer after they have finished their edit to clearly communicate their intent. To minimize trapdoor encounters, Parinfer can promote the use of a supplementary auto-closing quote feature found in most editors.

I created a wiki page if you want a more complete look at this problem and solution: https://github.com/shaunlebron/parinfer/wiki/The-Problem-with-Quotes

@shaunlebron
Copy link
Collaborator Author

oops, trapdoor idea was a bad mistake. I think we can just abandon processing if we encounter odd number quotes in a comment, which simplifies a lot. The mode function can return a warning allowing plugins to highlight the offending comment, which will avoid parinfer corrupting strings. I'll create some test cases to verify.

@shaunlebron
Copy link
Collaborator Author

closing this since the solution for #56 solved a general case for these kinds of problems

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

No branches or pull requests

3 participants