@NotTheDr01ds Please review this with care.

Thanks, can you add a test that the scope is only filled by those marked as export?

Thanks, can you add a test that the scope is only filled by those marked as export?

Sure, just figured out where to make that happen.

Oh, I should be more clear. The bug is that even exported constants shouldn't be available by calling use module.nu, but only accessible by $module.foo according to the nushell book. use module.nu * however behaves as what you'd expect.

Another interesting issue:

1. create a foo.nu file with the following content

export module "mod name" {
  export const var_name = "const value"
}

2. use foo.nu 'mod name'
3. invalid variable name in the scope:

Looks good. Does that issue appear also without #14920 applied? I.e., is it related to your changes, or is it just a bug in the const/module implementation?

Looks good. Does that issue appear also without #14920 applied? I.e., is it related to your changes, or is it just a bug in the const/module implementation?

Not related to #14920 , already the case in ver. 101 which is before that PR.

Basically space is not allowed in variable identifiers, but allowed in module names. So the problem arises when trying to reference a module as a variable.

TBH, I'm not a fan of such semantics, making lsp tasks more difficult, and the memory management is not well optimized.

The space comes from space being used as a submodule separator. We were thinking changing it to the more traditional :: which would eliminate the problem.

Can we make is_identifier_byte more strict? We could split on :: in normalize_module_name and parse_variable_expr to make $mod::name work.

Can we make is_identifier_byte more strict? We could split on :: in normalize_module_name and parse_variable_expr to make $mod::name work.

By $mod::name, do you mean the variable which is a record representing all the nested constants, or a single constant named name defined in a module named mod?

I thought if we choose to go for the :: syntax. We no longer need module record variables?

By $mod::name, do you mean the variable which is a record representing all the nested constants, or a single constant named name defined in a module named mod?

I mean the module record variable.

I thought if we choose to go for the :: syntax. We no longer need module record variables?

Sorry if I'm missing something obvious, but why not?

Sorry if I'm missing something obvious, but why not?

Could be my misunderstanding. I thought foo::bar would be a new type of expression especially designed for referring contents inside modules.

Currently if we execute use module.nu, all its exported commands would be available with a prefix module, while its exported constants stored in the module record, which are quite inconsistent semantics. With :: syntax however, use module.nu does nothing more than adding the module to the scope. And we can access all its contents with something like "mod name"::"command name", $"mod name"::"constant name".

The :: change is a substantial language change that still needs some design that's better done elsewhere, but yeah, I think it would allow us to move away from the module records.

Seems like the discussion has gone a little bit off topic but as it relates to this PR, I think we're ready to land this. Thanks!