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

Indenter and formatter fails while typing out body of deftype method #1957

Closed
wevre opened this issue Nov 9, 2022 · 11 comments
Closed

Indenter and formatter fails while typing out body of deftype method #1957

wevre opened this issue Nov 9, 2022 · 11 comments
Labels
bug Something isn't working editing formatting

Comments

@wevre
Copy link

wevre commented Nov 9, 2022

I expect the bodies of methods within deftype to be indented by 2 spaces and when I format the entire deftype form, they do indent properly. But while typing, the body is indenting all the way over to the second term.

(deftype MyType [arg1 arg2]
  IMyProto
  (method1 [this] |(print "hello world")))

Typing enter results in

(deftype MyType [arg1 arg2]
  IMyProto
  (method1 [this]
           |(print "hello world")))

It sort of flashes for a moment in the right spot, then scoots over. Hitting tab does not put it in the right spot. But I did find out something interesting while experimenting. If I hit enter and create a blank line

(deftype MyType [arg1 arg2]
  IMyProto
  (method1 [this]
           
           |(print "hello")))

and then hit backspace, it goes to the correct spot.

(deftype MyType [arg1 arg2]
  IMyProto
  (method1 [this]
    |(print "hello")))

Which is great! But if I hit tab again it will go over to the wrong spot. And as state above, if I format the entire deftype form, the bodies are indented correctly.

Originally posted by @wevre in #1956 (comment)

@PEZ PEZ changed the title Indenter fails while typing out body of deftype method Indenter and formatter fails while typing out body of deftype method Nov 9, 2022
@PEZ
Copy link
Collaborator

PEZ commented Nov 9, 2022

Copying comment from #1956 as well: The cause here is that Calva only formats the enclosing form, so deftype is never considered. backspace causes the parent form to be formatted, which is why your trick works. Similar if you format the deftype form.

@PEZ
Copy link
Collaborator

PEZ commented Dec 27, 2022

I think that the tools to solve this involve:

  1. The TokenCursor to identify when we're at the top level of a deftype method.
    • We have a function for determining the current defined function. This one can probably used, if we first move up out of the current enclosing form.
  2. The function for formatting the current position has a signature accepting extra options, of which one is depth. It defaults to depth 1, formatting only the current form.

So then when we format the current position we could check if we're on the top level of a deftype method and use depth 2 if so.

@SillyCoon
Copy link
Contributor

I'll try to resolve it, thanks @PEZ!

@SillyCoon
Copy link
Contributor

@wevre @PEZ I guess the issue is no longer valid. I've tested it manually and have added a unit test for this case
#2002

@PEZ
Copy link
Collaborator

PEZ commented Dec 29, 2022

@PEZ
Copy link
Collaborator

PEZ commented Dec 29, 2022

@SillyCoon, there are two indentation mechanisms in play for this. The one tested in your PR, which we can call the indenter, and also the formatter.

I don't understand why your test does not expose the problem with the indenter, but for the formatter it is clearer, and due to the depth issue I mentioned above.

@SillyCoon
Copy link
Contributor

Ok, got it, let me take a look at formatter

@bpringe bpringe added the bug Something isn't working label Jan 2, 2023
@SillyCoon SillyCoon mentioned this issue Jan 4, 2023
17 tasks
SillyCoon added a commit to SillyCoon/calva that referenced this issue Jan 4, 2023
SillyCoon added a commit to SillyCoon/calva that referenced this issue Jan 7, 2023
SillyCoon added a commit to SillyCoon/calva that referenced this issue Jan 7, 2023
SillyCoon added a commit to SillyCoon/calva that referenced this issue Jan 7, 2023
SillyCoon added a commit to SillyCoon/calva that referenced this issue Jan 8, 2023
SillyCoon added a commit to SillyCoon/calva that referenced this issue Jan 8, 2023
@PEZ
Copy link
Collaborator

PEZ commented Jan 20, 2023

This issue is fixed. We must have missed to include a closing commit.

@PEZ PEZ closed this as completed Jan 20, 2023
@PEZ
Copy link
Collaborator

PEZ commented Jan 20, 2023

That said, I am experiencing weird indentation issues quite often lately. Which could be regressions from the fixing of this issue. I'll try figure out a repro. It happens to me in my work project and code I can't share as is.

@SillyCoon
Copy link
Contributor

That said, I am experiencing weird indentation issues quite often lately. Which could be regressions from the fixing of this issue. I'll try figure out a repro. It happens to me in my work project and code I can't share as is.

Hi @PEZ! Plz tag me in the new issue when it'll be ready. I will try to fix it.

@PEZ
Copy link
Collaborator

PEZ commented Jan 25, 2023

That said, I am experiencing weird indentation issues quite often lately. Which could be regressions from the fixing of this issue. I'll try figure out a repro. It happens to me in my work project and code I can't share as is.

Hi @PEZ! Plz tag me in the new issue when it'll be ready. I will try to fix it.

Thanks, @SillyCoon! It's interesting, the repro for at least one of the strange things I've seen is super simple:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working editing formatting
Projects
None yet
Development

No branches or pull requests

4 participants