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

Issue 10302 - Package module conflicts with package name #2152

Merged
merged 3 commits into from
Jun 10, 2013

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jun 9, 2013

http://d.puremagic.com/issues/show_bug.cgi?id=10302

...and small fix up for #2139.

If the source tree is as follows:

pkg/
+ package.d
+ common.d

A package module will be incorporated to the internal package tree in two ways: import pkg; and import pkg.common;.

Current code considers either only one of them will occur. If both case occurs at the same time, like as import pkg; import pkg.common;, pkg as a Module and pkg as a Package will conflict each other.

To solve the problem, I added two fields isPkgMod and mod in Package class.

@@ -32,9 +32,19 @@
struct elem;
#endif

enum PKG
Copy link
Member

Choose a reason for hiding this comment

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

Please add comments on what these values mean.

@WalterBright
Copy link
Member

Kenji, your comments above are much better than the ones you put in the source code! Please make it the other way around.

@ghost ghost assigned WalterBright Jun 10, 2013
@andralex
Copy link
Member

Awesome, let's merge this soon.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 10, 2013

I updated comments in code.

Kenji, your comments above are much better than the ones you put in the source code! Please make it the other way around.

Just same description is already in the commit log.

enum PKG
{
PKGunknown, // not yet determined whether it's a package module or not
PKGmodule, // already determined that's an actual package module
Copy link
Member

Choose a reason for hiding this comment

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

i.e. package.d, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I rewrote "package module" to "package.d".

@WalterBright
Copy link
Member

This needs to go into the code as comments:

If the source tree is as follows:
  pkg/
  + package.d
  + common.d
A package can be incorporated into the internal package tree in two ways:
  import pkg; // pulls in pkg/package.d
  import pkg.common; // does not look at pkg/package.d
If both imports occur, pkg as a Module and pkg as a Package will conflict each other.
To fix this, we do [...]

Also, some explanation that a "package module" is "package.d".

Having these comments in the commit log isn't good enough, because when people try to understand the code they look at comments in the code, not in the commit log.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 10, 2013

Thanks for your reviewing. I updated the explanation comment and inserted it in the source code.

@WalterBright
Copy link
Member

Awesome! Thanks, Kenji.

WalterBright added a commit that referenced this pull request Jun 10, 2013
Issue 10302 - Package module conflicts with package name
@WalterBright WalterBright merged commit 54890d0 into dlang:master Jun 10, 2013
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.

3 participants