Skip to content

Fix file creation in quoted.QuoteCompiler #5487

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

Merged
merged 30 commits into from
Nov 22, 2018
Merged

Fix file creation in quoted.QuoteCompiler #5487

merged 30 commits into from
Nov 22, 2018

Conversation

michelou
Copy link
Contributor

"<quote>" is not a legal file name on Windows.
We suggest to use "_quote_" instead (similar to path "_site" in doc-tool, see Site.scala).

@@ -67,7 +67,7 @@ class QuoteCompiler extends Compiler {
*/
private def inClass(expr: Expr[_])(implicit ctx: Context): Tree = {
val pos = Position(0)
val assocFile = new PlainFile(Path("<quote>"))
val assocFile = new PlainFile(Path("_quote_"))
Copy link
Member

Choose a reason for hiding this comment

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

I think the problem is that we're creating a File at all. @nicolasstucki shouldn't this be a VirtualFile ?

Copy link
Contributor Author

@michelou michelou Nov 21, 2018

Choose a reason for hiding this comment

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

Good point. I forgot to mention the VirtualFile option.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, a virtual file would be better.

@smarter
Copy link
Member

smarter commented Nov 21, 2018

@michelou When you create a new branch, you should create it from origin/master:

git checkout origin/master -b new-branch

Otherwise you'll drag along all your commits form your current branch.

You can salvage the current branch by using an interactive rebase to pick only the commits you want to keep:

git rebase -i origin/master

This open a file with al the commits on this branch but not on master, you can just comment out the lines with the commits you don't want, then save the file. This rewrites the history of the branch. You can then git push --force to update the PR.

@michelou michelou changed the title Fix file name in quoted.QuoteCompiler Fix file creation in quoted.QuoteCompiler Nov 22, 2018
Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@@ -67,7 +67,7 @@ class QuoteCompiler extends Compiler {
*/
private def inClass(expr: Expr[_])(implicit ctx: Context): Tree = {
val pos = Position(0)
val assocFile = new PlainFile(Path("<quote>"))
val assocFile = new VirtualFile(Path("<quote>").path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not simply: new VirtualFile("<quote>")

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're right! Update follows.

@nicolasstucki
Copy link
Contributor

You still need to sign the Scala CLA

@allanrenucci allanrenucci merged commit 052c3b1 into scala:master Nov 22, 2018
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.

4 participants