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

handling auto-indent #59

Open
shaunlebron opened this issue Nov 25, 2015 · 17 comments
Open

handling auto-indent #59

shaunlebron opened this issue Nov 25, 2015 · 17 comments

Comments

@shaunlebron
Copy link
Collaborator

There are two extra features related to auto-indentation that Parinfer users are starting to expect. Parinfer may grow in scope to handle these cases somehow, or we will have to detail how this can be handled external to it:

  1. position of cursor after pressing enter (auto-indentation of newlines)
  2. auto-indenting a region of text according to specific rules (realigning according to conventions)

Specifically, @snoe and @SevereOverfl0w expect the following from item 1: when pressing enter at the end of the last line:

(defn foo [a b]
  (let [x (+ a b)]
    x))|

They expect the cursor to be at the following position:

(defn foo [a b]
  (let [x (+ a b)]
    x))
   |

This behavior is handled in vim by using a mode called autoindent (seen here), which trivially copies the indentation of the previous line.

The behavior for item 2 can be handled by vim-clojure-static, which defines configurable rules for how to indent forms.

These two modes conflict with each other inside vim, so a solution to this problem must combine them when appropriate.

@shaunlebron
Copy link
Collaborator Author

some extra links from @snoe for solving item 2:

@shaunlebron
Copy link
Collaborator Author

paredit.js uses the following rules (from computeIndentOffset):

  • special forms always indent two-spaces
  • indent one space if first line of parent sexp has one or zero tokens, or does not open with (
  • otherwise, indent to the start of the parent sexp's second token

@shaunlebron
Copy link
Collaborator Author

The plan is to add a pressedEnter: true flag to execute auto-indent for Indent or Paren Mode. It will have to be executed synchronously after enter is pressed, and requires the cursor option so we can auto-indent the correct line. Furthermore:

  • if previous sibling line exists, match its indentation
  • else, if next sibling line exists, match its indentation (will require retro-correction of result lines)
  • else, use default indentation of 2 spaces for () and 1 space for []{}

Lastly, in Paren Mode, we can auto-calculate cursorDx if pressedEnter: true in order to preserve relative indentation.

@Kurvivor19
Copy link

In regards to indentation, there are cases where tab is expected to be 8 spaces and opening file with parinfer on (in indent mode) messes it completely

@shaunlebron
Copy link
Collaborator Author

which parinfer plugin allows you to open a file in indent mode without processing in paren mode first?

@Kurvivor19
Copy link

parinfer-mode dor emacs.
To think of it, that sounds like a problem for the plugin

@rgdelato
Copy link

rgdelato commented Jul 6, 2017

I haven't had a ton of time to mess with it, so this is a bit of a knee-jerk reaction, but I'm not a fan of how Indent Mode can now easily get into a state where pressing the delete key seemingly doesn't work.

(def m {:a 1
        |})

From this state, the user can't delete, and instead they have to move the cursor somewhere else to kind of unlock themselves. I'm sure with some time, I'd get used to it, but it's definitely a bit weird.

EDIT: As mentioned in the other thread, I've been using Atom, with Parinfer for balancing parens along with Paredit solely for newline auto-indentation (which seems to work decently well). I don't know about Vim users.

@shaunlebron
Copy link
Collaborator Author

yeah, it's a bit of a stopgap solution to run Paren Mode when a line starts with a close-paren. It fixes a lot of edge cases that I can dig up if needed.

the new "smart" mode may be able to only auto-indent after a newline is inserted. Otherwise, we can revive the previewCursorScope option to allow you to backspace in that position.

Would you expect similar behavior for the following case?

(foo m {:a 1
        |} bar)

@rgdelato
Copy link

rgdelato commented Jul 6, 2017

Oh wow, that example is a tricky one. Maybe it could be possible for Parinfer to not let the user end up in that kind of a state at all? So I'm assuming that the code started as:

(foo m {:a 1|} bar)

...and the user pressed enter to end up at your example.

Would it work if when the user presses enter, instead of producing:

(foo m {:a 1
        |} bar)

...Parinfer produced something more indent-mode-ish, like:

(foo m {:a 1
        |}
       bar)

?

@shaunlebron
Copy link
Collaborator Author

shaunlebron commented Jul 6, 2017

your example is actually what I tried in Newline Mode, which I threw away in favor of Paren Mode.

The problem is that this edge-case occurs not just after pressing Enter but also for Backspace.

Paren Mode just allowed us to continue operating without splitting lines in surprising ways. It's a weird state that I couldn't resolve perfectly.

@shaunlebron
Copy link
Collaborator Author

shaunlebron commented Jul 6, 2017

@rgdelato there's not a lot of discussion happening in these issues, so i'm still patient to summarize past context, especially for someone who has good ideas around edge cases. please ask away if you have more ideas/questions. also helps me to reevaluate past decisions since my perspective is changing everyday

@oakmac
Copy link
Collaborator

oakmac commented Aug 11, 2017

... I'm not a fan of how Indent Mode can now easily get into a state where pressing the delete key seemingly doesn't work.

(def m {:a 1
        |})

I did some work using Smart Mode tonight and ran into this case. It is definitely weird to press Delete and not have the cursor move.

I think you may be able to account for this in the Parinfer API without requiring editor plugins to "send you which keys were pressed". I think with the changes array + cursor information you can detect if Delete (or other "remove one character behind the cursor") operation occurred and then adjust the result accordingly.

@oakmac
Copy link
Collaborator

oakmac commented Aug 11, 2017

Just confirmed that this behavior only happens when forceBalance is set to false.

@shaunlebron
Copy link
Collaborator Author

forceBalance just wipes out things when it can't handle them.

I could special case this by removing } on backspace:

(def m {:a 1
        |})

but that would be bad here:

(def m {:a 1
        |} bar)

@rgdelato
Copy link

rgdelato commented Aug 11, 2017

I asked on Slack about what should happen when the user presses "delete" from here:

(def m {:a 1
        |} bar)

@timgilbert suggested that this should happen:

(def m {:a 1|} bar)

which is an interesting idea. It's pretty clean: if you would go past a boundary with a leading close paren, jump to the previous line. I think it might be pretty cool if it doesn't end up feeling jarring in practice.


In a similar vein, this solution might also work?

(def m {:a 1}
       |bar)

if you would go past a boundary with a leading close paren, you lose the close paren and all contiguous whitespace after it. This also seems pretty clean, though maybe users would expect to get their close paren back if they press space from this result?

You could imagine a case where that would make sense, such as pressing "space" from here:

(def m '({:a 1}
         |) bar)

An alternate, but more complicated idea is that you'd temporarily end up here, with a close paren in a place that it clearly doesn't belong:

(def m {:a 1
       |} bar)

...but then after you move your cursor away, you'd be reformatted to this:

(def m {:a 1}
       bar)

This potentially introduces a weird state where the user can type (or paste) more stuff that's indented incorrectly:

(def m {:a 1
 :b 2|} bar)

Which I think might okay as long as we can fix them?

(def m {:a 1
        :b 2|} bar)

But I wouldn't be surprised if this last idea turns out to have some pretty rough edge-case implications and/or feel wrong when actually using it in practice.

@ohAitch
Copy link

ohAitch commented Dec 30, 2018

Some light necromancy

Is there a mathematical reason "at the beginning of line, followed by words" is treated differently from "not at the beginning of line, followed by words"? For me doing anything to the close-paren in response to a newline inserted before it(immediately or no) is a pretty clear violation of the visual language: paren trains are grey to represent "managed by parinfer", other parens are black to represent "not managed by parinfer", a black paren suddenly disappearing is jarring. This is probably also related to the current handling breaking undo-redo:

parinfer-undo-cropped

@gilch
Copy link

gilch commented Sep 6, 2024

As Parinfer works now (at least in Spacemacs), if I'm in this state,

(defn foo [a b]
  (let [x (+ a b)]
    x))|

and want to make a sibling for x, first I press backspace twice. Parinfer immediately replaces the trails, so it gets into this state:

(defn foo [a b]
  (let [x (+ a b)]
    x|))

I don't have to count, since the editor highlights the matching open bracket, I know which form a backspace will get me into.

Then if I hit enter, I'm still inside the let form. I feel like this work pretty well for me.

I could see the proposed autoindent behavior being usable once I get used to it, but it seems less nice compared to how it works now. However, it would still be pretty good if a shift-tab did a dedent to the next appropriate spot. Counting the right number of backspaces is a bit harder, especially if the parent open bracket is a ways up.

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

No branches or pull requests

6 participants