Skip to content

SI-9206 Fix REPL code indentation #4564

Merged
adriaanm merged 6 commits intoscala:2.11.xfrom
som-snytt:issue/prompt
Jun 22, 2015
Merged

SI-9206 Fix REPL code indentation #4564
adriaanm merged 6 commits intoscala:2.11.xfrom
som-snytt:issue/prompt

Conversation

@som-snytt
Copy link
Contributor

To make code in error messages line up with the original line of
code, templated code is indented by the width of the prompt.

Use the raw prompt (without ANSI escapes or newlines) to determine
the indentation.

Also, indent only once per line.

@scala-jenkins scala-jenkins added this to the 2.11.8 milestone Jun 18, 2015
@som-snytt
Copy link
Contributor Author

One test got by me because --update-check will accidentally make a check file for a SessionTest, which has the checkfile as it were embedded in the session transcript.

@som-snytt
Copy link
Contributor Author

/rebuild

Hoping it's one of those spurious IDE build failures.

@som-snytt
Copy link
Contributor Author

I guess not. I'd hate to bug him, but I wonder if @adriaanm knows if something around presentation compiler and OSGI has changed.

2015-06-18 23:52:33,531 DEBUG [main] - PresentationCompilerActivityListener - Starting PresentationCompilerActivityListener for project expr-eval-code-completion
2015-06-18 23:52:34,223 DEBUG [main] - SimpleContentProposalProvider$ - Cannot load returned type for someNull
com.sun.jdi.ClassNotLoadedException: Type has not been loaded
[snip]
2015-06-18 23:52:34,498  INFO [main] - SimpleContentProposalProviderIntegrationTest$ - Test SimpleContentProposalProviderIntegrationTest finished
2015-06-18 23:52:34,514  INFO [main] - ScalaProject - shutting down compilers for expr-eval-code-completion
2015-06-18 23:52:34,515 DEBUG [main] - PresentationCompilerActivityListener - Stopping PresentationCompilerActivityListener for project expr-eval-code-completion
2015-06-18 23:52:34,515  INFO [main] - ScalaPresentationCompiler - shutting down presentation compiler on project: expr-eval-code-completion
Tests run: 584, Failures: 0, Errors: 1, Skipped: 27, Time elapsed: 1,198.32 sec <<< FAILURE!

Results :

Tests in error: 
  javaVarArgConstructor(org.scalaide.debug.internal.expression.features.NewKeywordTest): Compilation failed

Tests run: 584, Failures: 0, Errors: 1, Skipped: 27

Maybe I'll just sneak by his office and see if the door is open. If he has a door.

@adriaanm
Copy link
Contributor

Who needs doors if they have headphones. Try rebasing? I think we just merged a tandem PR with scalaide related to this, but happy hour may be playing tricks on me.

Previously, handy `sys.BooleanProp.keyExists` ignored the
property value. While trying not to make any real estate puns,
this commit will let it go false if a value is supplied that
is not true in the usual Java sense. But what is truth?

Allows `scala -Dscala.color=off`, for example.
@retronym
Copy link
Member

Yep, rebasing will fix that one.

@som-snytt
Copy link
Contributor Author

This is the rebased result. Is there a command to issue like rebuild or sync? Sorry I'm out of the loop.

@retronym
Copy link
Member

A test failed in validate-test@bb361e7

% diff /home/jenkins/workspace/scala-2.11.x-validate-test/test/files/jvm/interpreter-jvm.log /home/jenkins/workspace/scala-2.11.x-validate-test/test/files/jvm/interpreter.check
@@ -366,5 +366,5 @@ plusOne: (x: Int)Int
 res0: Int = 6
 res0: String = after reset
 <console>:8: error: not found: value plusOne
-                plusOne(5) // should be undefined now
-                ^
+              plusOne(5) // should be undefined now
+              ^

@retronym
Copy link
Member

Scabot answers to these commands.

The scala shell prompt can be provided as either a system
property or in compiler.properties.

The prompt string is taken as a format string with one
argument that is the version string.

```
$ scala -Dscala.repl.prompt="%nScala %s> "
Welcome to Scala version 2.11.7-20150616-093756-43a56fb5a1 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_45).
Type in expressions to have them evaluated.
Type :help for more information.

Scala 2.11.7-20150616-093756-43a56fb5a1> 42
res0: Int = 42

Scala 2.11.7-20150616-093756-43a56fb5a1> :quit
```
To make code in error messages line up with the original line of
code, templated code is indented by the width of the prompt.

Use the raw prompt (without ANSI escapes or newlines) to determine
the indentation.

Also, indent only once per line.
Only for exactly `-Dscala.repl.info`, include the complete version
number string in the REPL prompt. One could imagine this is the mode
for posting snippets to stackoverflow.
@som-snytt
Copy link
Contributor Author

Thanks, Jason, now I see that I must check build history in Jenkins. I was dissuaded by the lack of links here.

I moved changing the default \nscala> to %nscala> to the next commit, that actually fixes indentation fix-up. The old assumption was to drop one char only from prompt; that test did manual intp.reset(), the .check was two chars off because of double indent; but it's not obvious to me why the output was different for only that one test case. TODO, perhaps. Also, hash tag things that shouldn't be hard to reason about.

@SethTisue
Copy link
Member

@adriaanm, should we go ahead and merge this for 2.11.7, or is it better to leave it alone for now given that REPL stuff is also getting torn up in #4563?

@adriaanm adriaanm modified the milestones: 2.11.8, 2.11.7 Jun 19, 2015
@som-snytt
Copy link
Contributor Author

The checks are always greener on the other side.

@adriaanm
Copy link
Contributor

They look pretty green from my desk, but I'm struggling to get 2.11.7 out the door in time as it is (to make matters worse the elevators are stopping on random floors today).

@som-snytt som-snytt closed this Jun 20, 2015
@adriaanm adriaanm modified the milestones: 2.11.7, 2.11.8 Jun 20, 2015
@adriaanm
Copy link
Contributor

Hope you don't mind my reopening and moving back to 2.11.7. I didn't make my deadline yesterday.

@adriaanm adriaanm reopened this Jun 20, 2015
@som-snytt
Copy link
Contributor Author

@adriaanm I was just fixing pasting with a custom prompt. About to push it.

@som-snytt
Copy link
Contributor Author

The intention of the previous code was to obviate the ctl-D by finishing a pasted transcript with a prompt. But there's a bug if there's more than one such empty line. EDIT: The bug was not detecting scala> by itself as a lone prompt. If you intend scala.> then you must omit the trailing space. Somewhere this commit fixes a trim.

Each empty line initiates replay, but next prompt is taken as an expression. Here res14 - res15 are pasted:

scala> 1
res14: Int = 1

scala> 

scala> 

scala> 2
res15: Int = 2

scala> scala> 1

// Detected repl transcript paste: ctrl-D to finish.

res14: Int = 1

scala> 
// Replaying 1 commands from transcript.

scala> 1
res16: Int = 1


scala> 

scala> scala> 
<console>:8: error: object > is not a member of package scala
              scala>
                   ^

scala> 

scala> scala> 2

// Detected repl transcript paste: ctrl-D to finish.

res15: Int = 2
// Replaying 1 commands from transcript.

scala> 2
res18: Int = 2

We talk about bit rot but not about how dust accumulates on
code that hasn't been swept since the last time the furniture
was moved around.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double negation could do with some De Morgan. !(assigned !"true") --> The key exists in the map and is either the empty string or "true".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to read the comment in a mirror. It's written from the standpoint of a falsifier, kind of like what do you ask a Cretan liar. How do I turn it off? Also, everyone knows that Father's Day is also Opposite Day.

@adriaanm
Copy link
Contributor

LGTM, with suggestions that hopefully will, erm, prompt a follow-up PR.

adriaanm added a commit that referenced this pull request Jun 22, 2015
SI-9206 Fix REPL code indentation
@adriaanm adriaanm merged commit 1fbce46 into scala:2.11.x Jun 22, 2015
adriaanm added a commit to adriaanm/scala that referenced this pull request Jul 2, 2015
`testBoth` cannot be a val in `Pasted`, as `Pasted` is inherited
by `object paste` in ILoop, which would cause `val testBoth` to
be initialized before `val PromptString` was:

```
object paste extends Pasted {
  val PromptString   = prompt.lines.toList.last
```

See https://scala-webapps.epfl.ch/jenkins/job/scala-nightly-checkinit-2.11.x/417.
Introduced by scala#4564.
@som-snytt som-snytt deleted the issue/prompt branch October 25, 2015 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants