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

server mode for visual debugging #215

Merged
merged 4 commits into from
Nov 5, 2024
Merged

server mode for visual debugging #215

merged 4 commits into from
Nov 5, 2024

Conversation

mimoo
Copy link
Contributor

@mimoo mimoo commented Oct 25, 2024

addresses #203

very primitive and clunky interface:

Screenshot 2024-10-25 at 1 54 35 PM

I run it this way in a noname project:

$ cargo run --manifest-path <noname_path>/Cargo.toml -- build --server-mode

the changes are pretty straight forward:

  • add --server-mode option to the build command
  • when enabled this runs a server in a thread, which also serves some index.html page on /
  • an optional channel sender is passed to the different compiler passes, which allows the different passes to send their state to the server
  • the server also has a channel to allow the compiler to pause and step only when the user wants to resume (that might be unnecessary, but heh)

@mimoo mimoo changed the base branch from main to feat/init-stdlib October 25, 2024 17:56
Copy link
Collaborator

@katat katat left a comment

Choose a reason for hiding this comment

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

Very nice feature to have! I left a comment about simplifying the passing of server_mod argument.

src/imports.rs Outdated Show resolved Hide resolved
src/imports.rs Outdated Show resolved Hide resolved
src/server/mod.rs Outdated Show resolved Hide resolved
src/server/mod.rs Outdated Show resolved Hide resolved
src/server/mod.rs Outdated Show resolved Hide resolved
@@ -116,6 +130,9 @@ pub fn cmd_build(args: CmdBuild) -> miette::Result<()> {

println!("successfully built");

// server mode
handle.map(|h| h.join().unwrap());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we don't use the return from the map function, maybe be better to this instead:

Suggested change
handle.map(|h| h.join().unwrap());
if let Some(h) = handle { h.join().unwrap() }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what's the difference?

Copy link
Collaborator

Choose a reason for hiding this comment

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

then clippy won't throw warning :)

@@ -184,7 +205,7 @@ fn produce_all_asts<B: Backend>(path: &PathBuf) -> miette::Result<(Sources, Type
let mut tast = TypeChecker::new();

// adding stdlib
add_stdlib(&mut sources, &mut tast, node_id)?;
add_stdlib(&mut sources, &mut tast, node_id, server_mode)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe it is better to create server_mode fields in nast/tast/mast structs? so we don't have to add server_mode as an argument everytime we call the functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think we should have some sort of super Options struct so that we can manage all of these arguments that we pass in different places, and have it have a Default implementation as well as setters

src/lexer/tokens.rs Outdated Show resolved Hide resolved
src/parser/mod.rs Outdated Show resolved Hide resolved
src/compiler.rs Outdated Show resolved Hide resolved
@mimoo
Copy link
Contributor Author

mimoo commented Oct 29, 2024

I'll wait for you @katat to merge the init-stdlib PR first

@katat
Copy link
Collaborator

katat commented Oct 30, 2024

init-stdlib is already merged. I think you need to change the base branch.

@mimoo mimoo changed the base branch from feat/init-stdlib to main November 5, 2024 12:20
@mimoo mimoo merged commit 2f4340e into main Nov 5, 2024
1 check failed
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.

2 participants