Clojurians Log v2

Ok, no problem. I don’t think the changes are that conceptual to be honest, but there’s a lot of changes to all the bookkeeping which needs some thought.

Assuming this looks mostly ok, I’m going to get it into a dev build of Cursive people can try - there are too many cases I can’t simulate in CodeMirror (renaming vars etc)

@doglooksgood Here’s an alpha version of what I was talking about last week: https://github.com/cursive-ide/parinfer/tree/process-buffer

Here is how it integrates into CodeMirror: https://github.com/cursive-ide/parinfer/blob/process-buffer/site/resources/public/js/lib/parinfer-codemirror.js#L478-L533

Here is the main loop - this is where you would integrate whatever Emacs offers for a char iterator over the buffer: https://github.com/cursive-ide/parinfer/blob/process-buffer/lib/parinfer.js#L1296-L1316

@cfleming: all tests are passing?

No, I’m trying the test cases in the editor. The one we discussed the other day doesn’t actually seem to be an issue with the new functionality to warn on unbalanced close parens.

and tangentially: is the race condition in Cursive that prevents files from being pre-processed by paren mode fixed?

No, I’m planning to look at that at the same time as fixing this.

my survey showed that to be a problem

It’s definitely a problem.

seems critical honestly

Yup.

in the #clojurescript channel, i think it’s more than a few people’s perception that parinfer can break your code if not indented correctly

could there be a redundant safety check to not run indent mode on a file until paren mode does?

I’d need to go back and look at the details of how I implemented that. I remember it was hard, but I don’t remember all the details.

this is the comment I saw: https://clojurians.slack.com/archives/C03S1L9DN/p1501185363764216

So the second failing test case also appears to not be an issue in the actual editor, I’m not sure why.

i have to step out for an hour

I lie, it is a problem, Chrome had cached the old version.

Can I change buffer step by step, instead of process-text -> public-result? I am not very familiar with the algorithm for now.

Yes, that is what the edit changes tries to do. It’s not fully working yet, though.

I fixed the last smart mode bug, and a bug in the CodeMirror integration. After some initial playing, it seems good now.

The indent mode test is still failing, and I don’t have a good solution for that. I’ll file an issue with the details from earlier in this chat so they don’t get lost.

There’s one issue - I no longer return the new cursor value from parinfer in this version, and just rely on CodeMirror to push the caret around based on the edits. There’s one very common case where that’s a problem: entering new parens.

So if open a new paren, I end up with ()| because of the way that the edits are created. This isn’t a problem in Cursive because I explicitly create both parens and put the caret between them - I suspect most actual editors will do that.

BTW that issue is just an edge case in what to do when an edit inserts exactly at the caret - it could be worked around in CodeMirror too with no problem.

I think a article in detail how this algorithm works will help a lot, since every editor have different plugin system.

@doglooksgood Do you mean parinfer itself?

yes，I want to rewrite to let parinfer process buffer directly. not a function to function port.

i will have to update this document with smart mode changes: https://github.com/shaunlebron/parinfer/blob/master/lib/doc/code.md

@doglooksgood it doesn’t make sense to move forward on documenting and porting something that we are not yet sure people will like

@doglooksgood That is exactly what the branch/fork that I sent you does - if you want to do something different to that, I don’t understand what you mean.

That change may become parinfer mainline at some point, but that’s not decided yet.

FWIW this is one of the best-documented projects I’ve ever been involved with. There are conceptual docs in the webpages, the code is well documented and there is a lot of very useful discussion in the issues. The doc isn’t currently up to date with smart mode, but like Shaun says that’s still experimental.

It is a complicated algorithm now, and the code takes some time to understand, but it’s doing a lot these days.

It’s probably not something you can port to elisp in a weekend.

@chrisoakman Now that Atom has smart mode, do you still expose Indent and Paren mode to the user?

Are you actively using it at the moment? Do you have a feeling for whether they’re still necessary?

Or rather, will be if smart mode works out?

Anyone else using the latest Atom version have opinions on that?

The options in Atom now have a checkbox for replacing Indent Mode with Smart Mode (with Smart Mode off by default), so the user can still use Indent Mode

@rgdelato If you have smart mode, do you think you’d still need indent mode?

probably not? But I think they're hedging because Smart Mode has some behavior that people might not like

Sure, I understand it’s still experimental, just interested in the experiences of people actually using it.

If Indent Mode had at least kept the cursorDx thing, then maybe it'd be kind of a choice for me, but since Smart Mode is the only one that currently preserves the structure underneath my edit, I'm 100% Smart Mode all the way

That’s good to hear, thanks!

Probably my biggest complaint with the current Smart Mode is that you can't delete from certain states: https://github.com/shaunlebron/parinfer/issues/59#issuecomment-313469378

(which is like multiple orders of magnitude smaller than the previous complaint of "I need to switch modes when making certain edits", so still a huge win)

Interesting, that is tricky.

I’m hoping to get a dev build of Cursive out with smart mode soon so people can test it.

@cfleming I haven't been using atom-parinfer enough this week to really have an opinion on smartMode. I'm generally bullish on it vs Indent Mode. But still evaluating it from usage.

Like Ryan said, atom-parinfer basically still works the way it has always worked. Except now there is an option to use smartMode instead of Indent Mode.

The plan is to remove Indent Mode and only have Smart Mode once it is stable / awesome.

atom-parinfer v2.0.0 should hopefully be Smart Mode by default

I think Paren Mode will always exist in some form. Even if only to run on the file when it is first opened.

Yeah, I can definitely see that the modes need to exist internally, but was curious if users still had a need for them.

@doglooksgood I definitely want to keep parinfer-elisp up-to-date with parinfer.js. Have been waiting for it to stabilize.

Also I read that thing about "buffer is faster than string" for the elisp docs. I went with strings for port clarity. I wonder how much the speed difference really is? In general, I would be wary of going that route unless we had some good metrics showing the difference between the two.

All that said, I am not an emacs user. And not opposed to that approach if it is better. It's just important that the ports all pass the same test suite as parinfer.js

@cfleming I don't really know how confusing the two modes are for users. Ideally: there would be no modes. But I think in practice people are in Indent (or now Smart) mode 99% of the time.