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

x/tools/cmd/present: improper handling of "pressing enter" while editing #41139

Closed
Hamza141 opened this issue Aug 30, 2020 · 2 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@Hamza141
Copy link

What version of Go are you using (go version)?

go version go1.15 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Hamza\AppData\Local\go-build
set GOENV=C:\Users\Hamza\AppData\Roaming\go\env  
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Hamza\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Hamza\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=D:\Projects\tools\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=R:\Temp\go-build900148230=/tmp/go-build -gno-record-gcc-switches

What did you do?

Given the following .slide file

Let's break stuff

* Time to break
.play -edit break.go

And the following go file

package main

import (
	"fmt" // this is a comment
)

func main() {
	fmt.Println("Hello world!")
}

Running go present and navigating to the second slide with the editable code.

Importing time in the slide while presenting like so

package main

import (
	"fmt" // this is a comment
        "time"
)

func main() {
	fmt.Println("Hello world!")
}

And then running the code.

What did you expect to see?

There should be an error because time is not in use.

What did you see instead?

It still shows the Hello world! output.

Further explanation (with pics)

I investigated this further and it looks like the issue is on the html side.

Here's how the html looks like initially
image

Pressing enter in the slide code adds another <pre> tag that essentially splits the code
image

Typing "time" doesn't add a new span for it
image

This results in "time" getting interpreted as being on the same line as the comment (// this is a comment).
Therefore, the snippet complies and runs.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Aug 30, 2020
@gopherbot gopherbot added this to the Unreleased milestone Aug 30, 2020
@gopherbot
Copy link

Change https://golang.org/cl/251697 mentions this issue: present: fix newline parsing for go present

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 2, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Sep 3, 2020

I can reproduce the issue in Chrome and Safari. Relying on contenteditable="true" for this seems prone to problems, and I would guess browsers changed its implementation at some point so it no longer inserts <BR>s. But that code was written very long ago. Also handling the "PRE" tag seems reasonable, let's do it for now before trying to do more.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 3, 2020
@golang golang locked and limited conversation to collaborators Sep 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants