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

Should insert_after & insert_before clobber at the same insertion point? #400

Closed
marcandre opened this issue Nov 19, 2017 · 2 comments
Closed
Labels

Comments

@marcandre
Copy link
Contributor

marcandre commented Nov 19, 2017

I feel that insert_before and insert_after should be revised in light of #399.

In particular, the following code currently raises clobbering errors:

      @rewriter.
        insert_after(range(5, 1), '>').
        insert_before(range(6, 0), '[').
        insert_after(range(6, 0), ']').
        insert_before(range(6, 1), '<').
        process

I feel that the code above should be allowed without errors. One argument is that the following code is equivalent but does not raise a clobbering error:

      @rewriter.
        replace(range(5, 1), "#{range(5, 1).source}>").
        replace(range(6, 0), "[#{range(6, 0).source}]").
        replace(range(6, 1), "<#{range(6, 1).source}").
        process

The above examples should both produce 'foo ba>[]<r baz.

In short, I'd suggest that there should be no cloberring error as long as there's at most one of each insert done on a non-empty range, and at most one of each done on an empty range (like in the example above).

Let me know if I should provide a PR for this.

@whitequark
Copy link
Owner

@alexdowad @JoshCheek thoughts?

@alexdowad
Copy link
Collaborator

This makes a lot of sense. Thumbs up!

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

No branches or pull requests

3 participants