Trailing comma in imports

[lens0021]

Ignore trailing commas in imports.

I've used GH-123 as a reference and given ChatGPT's advice.

[hdwalters]

Approved with some minor suggestions.

[hdwalters]

This token check is only required on the first iteration through the loop, to catch an empty import list. I believe that on all subsequent iterations, any trailing brace is caught by the other check below.

It won't make any noticeable difference to the performance, but it may help subsequent developers understand the logic better, if you move the token check out of the loop. You might also change vec![] to Vec::new():

token(meta, "{")?;
let mut exports = Vec::new();
if token(meta, "}").is_err() {
    loop {
        let tok = meta.get_current_token();
        let name = variable(meta, variable_name_extensions())?;
...

[b1ek]

You might also change vec![] to Vec::new()

why? we use vec![] throughout the codebase. there is no reason to have an inconsistency like this

[hdwalters]

Okay, fair enough. In isolated code, I would prefer a straight Vec::new() call because simpler is better, and you really only need the vec![] macro to initialise an array from a sequence of items. However, I'm all for internal consistency in a project; happy for it to remain as-is.

[lens0021]

All test passed now, but I am not sure the code I've written is clean. Feel free to suggest changes.

[b1ek]

your code is fine

[Ph0enixKM]

Well done! I came up with an idea to add a warning when nothing is actually imported

[Ph0enixKM]

A warning for developers to quickly spot an empty import statement:

import { } from "std/text"

With a warning message like:

Suggested change

	}
	} else {
	let message = Message::new_warn_at_token(meta, Some(self.token_import.clone()))
	.message("Empty import statement")
	meta.add_message(message);
	}

[lens0021]

Committed with some modification: i) Adding semicolon, and ii) Removing Some(). (E0308: expected struct heraclitus_compiler::compiling::Token, found enum Option<heraclitus_compiler::compiling::Token>)
And I have a question. Is there a way to make a unit test for warning?

[Ph0enixKM]

Uhhh not for now but that should be implemented at some point. I'll make an issue about it

[hdwalters]

Won't this empty import now generate a warning? If so, please remove.

[Ph0enixKM]

Thanks, @lens0021