brownplt / pyret-lang

The Pyret language.
Other
1.07k stars 110 forks source link

Indentation is not enforced #2

Open jpolitz opened 11 years ago

jpolitz commented 11 years ago

Pyret's design explicitly asks for indentation enforcement. It isn't implemented yet.

schanzer commented 7 years ago

Since this is really old, I just wanted to check in: does this mean whitespace-sensitivity? And if so, is that still part of the design plan?

shriram commented 7 years ago

Whitespace awareness rather than sensitivity. That is, whitespace is checked to be correct, but it does not imply anything for the meaning of the program (you could collapse all the spaces into one, and that's fine).

It's still a future feature.

blerner commented 7 years ago

This should be very easy for someone to pick up and do, and would be a good beginner bug, since it doesn't try on Pyret code at all. (At the very least, a first draft would be a function that just returned a Boolean of whether the Pyret mode indenter would take the current file and leave it unchanged. A second draft would return a list of line numbers whose contents aren't indented correctly.) The main thing to decide is the UI for how to report errors to the user. (Red squiggles, icons in the left gutter, or actual error messages in the repl, or something else entirely...)

SethPoulsen commented 6 years ago

Just a couple quick questions before I take a crack at this:

  1. Should the complier issue an error if the whitespace isn't correct, or just a warning?
  2. The Pyret mode is in a different repository--what is the best way to reference/reuse that without code duplication?
schanzer commented 4 years ago

@SethPoulsen did you ever make a run at this?

SethPoulsen commented 4 years ago

@schanzer I never did get a chance to do it and unfortunately its not something I have time for at the moment.

safwansamsudeen commented 1 year ago

Woah this issue is old!

@jpolitz Can I work on it? It'd be great if Seth's doubts could be clarified.

schanzer commented 1 year ago

@safwansamsudeen this would be a HUGE help to Bootstrap students, teachers, and facilitators!

safwansamsudeen commented 1 year ago

@schanzer great to hear!

I'm new to the language, so I'll take quite some time 🙂.

Again, should the complier issue an error if the whitespace isn't correct, or just a warning?

blerner commented 1 year ago

Pyret doesn't have warnings, currently: either something is bad enough that we have Strong Opinions about it (and make it an error that prevents the program from running), or we don't care too strongly about it and let it slide. Whitespace indentation does not affect the semantics of the program, and fixing it in CPO is just a matter of select-all-then-press-tab. @schanzer do you really want to prevent students in the very first hour of coding to let ugly-but-valid programs from running? It's a lousy first-run experience...

(Then there are also semantic difficulties in "enforcing" indentation for multi-line comments and strings. I've never figured out a single, definitive solution there, so I'm glad we haven't enforced anything about their indentation yet... Those don't tend to show up in BS:* much, but they still do matter to our other target audiences.)

schanzer commented 1 year ago

@blerner I was thinking that "enforcement" would come at the CodeMirror level -- we just re-run autoindent on every keystroke (throttled by something reasonable).

100% agree no compilation errors!

blerner commented 1 year ago

Reindenting on every keystroke is expensive and jarring: the lines will bounce around even more than they currently can do (e.g. if you have a variable named end-something, the indentation can get messed up right after the d). We already do it on every newline, to make sure the next line is correct. It takes explicit effort by someone to make the default indentation wrong (or if they have a variable named end-something, annoyingly...). So... When do you want to have the indentation happen?

schanzer commented 1 year ago

I'd say either:

  1. 2-5sec after the last keystroke
  2. When the user clicks Run

My person vote is for the latter.

blerner commented 1 year ago

So I'll admit I'm very leery of any automatic Reindent-All commands, as a blanket concern, because of those multiline comments and strings: reindent on them does not and cannot work consistently, so automatic reindentation is a mess. (Start from the common case of "I want to start a block comment for documentation." By default, the body of that comment should be indented 3 spaces beyond the opening #|. As you write the comment, each time you press enter, it automatically indents that line for you. But then consider the other common case of "I'm trying a bunch of code, and it doesn't work. I want to comment out a big block of it, and work on another section." Reindent-All would smash all that code to 3 spaces past the left margin. True, it has no semantic impact and when you uncomment it, it can be reindented again, but it's distracting and confusing and ugly to have that happen at all. Multiline strings are even worse, because line-leading whitespace is part of the string -- see https://github.com/brownplt/pyret-lang/issues/639 for long-ago unresolved discussion about that nuisance...)

I'm also leery of the Run button doing anything other than running your program. If a student's attention has shifted from the left side of the screen to the top-right (for the Run button) and then to the right side (for the REPL), then it'll be either distracting to see motion out the corner of their eye, or confusing to look back to the left and have their code be different-looking than it was a moment ago.

I'm pretty sure this issue was opened before CPO and CodeMirror had a good auto-indenter, so I don't honestly know if this issue is still needed. My gut sense is that this will kick in infrequently (because the current indenter is pretty darn good), which will make the few moments when this does do something feel even more disconcerting and unpredictable.

@schanzer you said this would be a "HUGE" help -- honestly, in what scenarios? When do students/teachers/facilitators get tripped up by the current indenter and lack of "enforcement"?

safwansamsudeen commented 1 year ago

Speaking purely from my perspective, indentation was a really annoying thing in Python when I started off in programming. Arcane errors, annoying forced code changes - but then, you don't want a compiler error.

As I see it, you want to enforce indentation but not raise a compiler error. That doesn't seem right to me - you either enforce it or you don't. Am I missing something here?

schanzer commented 1 year ago

@safwansamsudeen yes, I want to enforce indentation. But I think it's too black and white to say that enforcement only happens via compiler errors. A few comments back you were thinking about warnings, right?

@blerner the number of teachers and students who misspell example:, forget an end, or miss a closing paren or bracket is fairly high, especially in the early days of the PD or weeks of the class. More interestingly, there's a small core of students and teachers who never seem to become more precise about syntax, so there's this super long tail of frustrated people who are getting the kitchen-sink error that something might be missing or something might be there when it shouldn't be.

The PD solution is to show them how to select-all and hit the tab key, but that relies on them having an intuition for what code should look like. Until they have a sense for indentation, it's useless for them to scroll through freshly indented code looking for "where it goes wrong". So we wind up needing to re-indent all over and over again until they start getting a sense for how it should look.

Given them near-constant exposure to indenting would accelerate this process enormously, and let us point it out immediately in PD rather than waiting for teachers to get stuck.

Another option would be use CodeMirror to merely hint at bad indentation. For example, displaying a small gray [tab] widget at the end of lines that are poorly indented, or using the gutter somehow. That way there's a visual cue without there being any warnings or code changes. Anyone who hovers over the cue gets a popup suggesting that indentation is incorrect, and recommending they select all and hit tab to reindent.

If suggestions along these lines don't feel right either, then I recommend closing this as wontfix.

shriram commented 1 year ago

I don't think this should be a wontfix — figuring out indentation has been a todo since the first days of the language — but I also don't think overly simplistic solutions will help here. Let's understand all dimensions of the problem before we propose solutions.

Note that doing this automatically will also annoy people who don't like the default indentation everywhere and explicitly do something different [ahem, me].

I hate to suggest modes, but it sounds like we have at least two different kinds of users, one kind who really need a lot of help and another who would find it very annoying, and the transition comes after a little bit of experience with the language. So arguably it could start out in a very unforgiving mode, but the user can flip a bit that basically says "stop doing that, I'll do it manually when I need it". I would even be interested in having that on by default for myself and seeing how it goes for a while.

blerner commented 1 year ago

@schanzer I do not see how Reindent-All-Always is going to affect that pain point. You already have the lack of indentation (if examples: is misspelled), and you already have the continued indentation (if end is missing). Are you saying that students/teachers spend more time adding/removing whitespace "because Pyret is wrong" than actually reading their code? If so, indentation isn't going to help them; they're focused on the wrong thing. (You also have all the syntax-highlighting aides, of red-highlighted unmatched delimiters and the non-bolded exmaples typos or whatever -- do they not see those either?) Here's a dumb suggestion: what if we just changed the error feedback to say "Hint: try reindenting all your code, to see where it starts to look weird."? That still won't fix things (because the indenter won't let indentation drop below zero, so exmpales: ... end won't set the indentation to -1 and smush all subsequent code to the left margin).

@shriram I don't understand what you're proposing, though. There are at least three possible approaches: (1) the status quo, of auto-indent-each-new-line-of-code-on-Enter, (2) do nothing, (3) reindent-everything-always. You seem to be advocating for 2 (for you/power-users/people-who-don't-like-the-current-style) and 3 (for novices).

schanzer commented 1 year ago

@blerner adding "try reindenting your code" to the error would be hugely helpful, especially if it includes a link/button that does this automatically without having to memorize key conventions.

As for @shriram 's comment, this is yet another possible use-case for a "mode" feature, along with optionally-truncated display of numbers, type checking, and at least one or two other settings we've brainstormed over the years.

safwansamsudeen commented 1 year ago

Hey,

I actually don't know Pyret, and thought this would be a simple issue that I could start off with to apply my newly-found knowledge. It actually seems quite complicated, and the recent discussion makes the resultant PR even more so. So I think I'd better unassign myself from this.

Thanks for your help! Sorry if this resulted in a lot of discussion, but I'm sure clearing this issue (no. 2 in the lang!) is a good idea anyway.

blerner commented 1 year ago

No need to apologize! It's useful to be reminded to look over the old issues (and lingering unresolved design questions) every now and then :⁠-⁠)

shriram commented 1 year ago

Loosely related: TIL F# actually has two syntaxes, "lightweight" and "verbose", related to indentation:

https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/verbose-syntax

There are two forms of syntax available for many constructs in F#: verbose syntax and lightweight syntax. The verbose syntax is not as commonly used, but has the advantage of being less sensitive to indentation. The lightweight syntax is shorter and uses indentation to signal the beginning and end of constructs, rather than additional keywords like begin, end, in, and so on. The default syntax is the lightweight syntax. This topic describes the syntax for F# constructs when lightweight syntax is not enabled. Verbose syntax is always enabled, so even if you enable lightweight syntax, you can still use verbose syntax for some constructs.