Skip to content

reference path with quotes doesnot have same result as in old compiler #474

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

Closed
sheetalkamat opened this issue Aug 18, 2014 · 8 comments · Fixed by #605
Closed

reference path with quotes doesnot have same result as in old compiler #474

sheetalkamat opened this issue Aug 18, 2014 · 8 comments · Fixed by #605
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@sheetalkamat
Copy link
Member

New compiler:
tsc tests\baselines\reference\project\quotesInFileAndDirectoryNames\node\m'ain.d.ts

Error:
tests/baselines/reference/project/quotesInFileAndDirectoryNames/node/m'ain.d.ts(1,1): File 'tests/baselines/reference/project/quotesInFileAndDirectoryNames/node/li.ts' not found.

Old compiler:
No error

Note we either need to fix the reference path resolution for the quotes, or fix the declaration emitter to handle the quotes correctly in reference path

@mhegazy
Copy link
Contributor

mhegazy commented Aug 18, 2014

quotes in file names are accepted by the file system so there is no reason to disallow them.

@mhegazy mhegazy added this to the TypeScript 1.1 milestone Aug 18, 2014
@mhegazy mhegazy added the Bug label Aug 18, 2014
@sheetalkamat
Copy link
Member Author

Old compiler could parse the below reference as li'b/class'A.d.ts

/// <reference path='li'b/class'A.d.ts' />
declare class ClassC extends test.ClassA {
}

New one parses this as li

@mhegazy we want to behave like old compiler here right? Or should user be writing the reference as "li'b/class'A.d.ts" instead?

@mhegazy
Copy link
Contributor

mhegazy commented Aug 23, 2014

@sheetalkamat "Li'b/class'A" looks more correct.

@DanielRosenwasser
Copy link
Member

Doesn't this basically mean that 'reference' tags are limited to a 'path' attribute? Kind of defeats the purpose of even having made it a tag.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 24, 2014

@DanielRosenwasser makes sense. Then maybe we should check for valid XML attribute.

@CyrusNajmabadi
Copy link
Contributor

That’s not legal XML, so we shouldn’t support it. However, it’s also a breaking change, so we must support it.

         -- Cyrus

From: Sheetal Nandi [mailto:[email protected]]
Sent: Friday, August 22, 2014 3:23 PM
To: Microsoft/TypeScript
Subject: Re: [TypeScript] reference path with quotes doesnot have same result as in old compiler (#474)

Old compiler could parse the below reference as li'b/class'A.d.ts

/// <reference path='li'b/class'A.d.ts' />

declare class ClassC extends test.ClassA {

}

New one parses this as li

@mhegazyhttps://github.com/mhegazy we want to behave like old compiler here right? Or should user be writing the reference as "li'b/class'A.d.ts" instead?


Reply to this email directly or view it on GitHubhttps://github.com//issues/474#issuecomment-53128355.

@DanielRosenwasser
Copy link
Member

Then let's "deprecate" that style by parsing it the right way, and falling back to the old behavior if that fails. When we have to default to the old behavior, issue a warning.

The biggest problem is that we don't have a good dichotomy for warnings and errors - so really we'd like to warn in the parser, but instead we fail hard as soon as we issue any sort of diagnostic in the parser.

@DanielRosenwasser
Copy link
Member

Because it seems like a bug in the old compiler, and even if it was intentional, I don't believe it was right. If we're entirely certain that no attributes will be added, then by all means, it doesn't matter, but it just gives more weight to suggestions like #488.

sheetalkamat added a commit that referenced this issue Sep 4, 2014
…that quotes in file name wouldnt affect reference resolution

Fixes #474
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants