Skip to content

Added loading std from any location #19

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.

Sign up for GitHub

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

Open
farkon00 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added loading std from any location #19

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,15 +588,32 @@ impl Context {
Ok(())
}

fn process_file(&mut self, loc: Loc, file_path: String, diag: &mut impl Diagnoster) -> Option<()> {
let source = match fs::read_to_string(&file_path) {
Ok(source) => source,
fn load_source(&self, loc: Loc, file_path: &str, diag: &mut impl Diagnoster) -> Option<String> {
match fs::read_to_string(file_path) {
Ok(source) => Some(source),
Err(err) => {
diag.report(&loc, Severity::Error, &format!("could not load file {}: {}", file_path, err));
return None
None
}
}
}

fn process_file(&mut self, loc: Loc, file_path: String, diag: &mut impl Diagnoster) -> Option<()> {
let source = if file_path.starts_with("std/") {
match env::var("NOQ_STD_PATH") {
Ok(std_path) =>
self.load_source(loc,
&(std_path + file_path.strip_prefix("std/").unwrap()), diag),
Err(_) => self.load_source(loc, &file_path, diag)
}
}
else {
self.load_source(loc, &file_path, diag)
};
Comment on lines +602 to 612

@Noratrieb Noratrieb Nov 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This could be rewritten in several ways. We can get rid of the unwrap by using strip_prefix in the if instead of starts_witth:

        let source = if let Some(stripped_path) = file_path.strip_prefix("std/") {
            match env::var("NOQ_STD_PATH") {
                Ok(std_path) =>
                    self.load_source(loc, 
                        &(std_path + stripped_path), diag),
                Err(_) => self.load_source(loc, &file_path, diag)
            }
        } else {
            self.load_source(loc, &file_path, diag)
        };

We also see the pattern of looking whether the env var is found and using it and if isn't found we fall back to the default value. We could use the unwrap_or method instead

        let source = if let Some(stripped_path) = file_path.strip_prefix("std/") {
            let path = env::var("NOQ_STD_PATH")
              .map(|std_path| std_path + stripped_path)
              .unwrap_or(file_path);

            self.load_source(loc, &path, diag)
        } else {
            self.load_source(loc, &file_path, diag)
        };

Now there's one more if we can eliminate

        let path = file_path.strip_prefix("std/")
          .and_then(|stripped_path| env::var("NOQ_STD_PATH").map(|std_path| std_path + stripped_path))
          .unwrap_or(file_path);

        let source = self.load_source(loc, &path, diag).load_source(loc, &file_path, diag);

Is it better? At this point I'm not fully sure whether it's better and maybe the second one is better than the third, but I did have fun writing it :D

Also there could be minor errors in my code as I just wrote it in the GitHub UI and didn't check it but the general idea should work

let mut lexer = Lexer::new(source.chars().collect(), Some(file_path));
if source.is_none() {
return None;
}
Comment on lines +613 to +615

@Noratrieb Noratrieb Nov 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if source.is_none() {
return None;
}

The ? operator below already returns None if source is None, so this extra check is not needed.

let mut lexer = Lexer::new(source?.chars().collect(), Some(file_path));
while lexer.peek_token().kind != TokenKind::End {
self.process_command(Command::parse(&mut lexer, diag)?, diag)?
}
Expand Down