Adjusted code to PR suggestions and added some comments

cider-dap/src/adapter.rs

@@ -78,8 +78,8 @@ impl MyAdapter {
     pub fn create_stack(&mut self) -> Vec<StackFrame> {
         let frame = StackFrame {
             id: self.stack_count.increment(),
-            // TODO: edit name field
-            name: String::from("Hi"),
+            // Maybe automate the name in the future?
+            name: String::from("Frame"),
             source: Some(Source {
                 name: None,
                 path: Some(self.source.clone()),
@@ -109,23 +109,24 @@ impl MyAdapter {
     }
 
     pub fn next_line(&mut self, _thread: i64) -> bool {
+        // Step through once
         let status = self.debugger.step(1).unwrap();
 
         // Check if done:
         if status.get_done() {
+            // Give bool to exit the debugger
             true
         } else {
-            let map = status.get_status().clone();
-            // Declare line number beforehand
+            let map = status.get_status();
             let mut line_number = 0;
-            // Return 0 should a lookup not be found. This really shouldn't happen though.
             // Implemented for loop for when more than 1 group is running,
-            // the code for now goes to the last group running in the map, should deal
-            // with this in the future.
+            // the code for now goes to the line of the last group running in the map, should deal
+            // with this in the future for when groups run in parallel.
             for id in map {
                 let value = self.ids.lookup(id.to_string()).unwrap().line;
                 line_number = value;
             }
+            // Set line of the stack frame and tell debugger we're not finished.
             self.stack_frames[0].line = line_number as i64;
             false
         }

cider-dap/src/main.rs

@@ -148,7 +148,8 @@ where
     // Construct the adapter
     let mut adapter = MyAdapter::new(program_path, std_path)?;
 
-    //Make two threads to make threads visible on call stack, subject to change.
+    // Currently, we need two threads to run the debugger and step through,
+    // not sure why but would be good to look into for the future.
     let thread = &adapter.create_thread(String::from("Main"));
     let thread2 = &adapter.create_thread(String::from("Thread 1"));
 
@@ -199,7 +200,6 @@ fn run_server<R: Read, W: Write>(
                 }
             }
 
-            // TODO: Implement this request fully when adapter becomes functional
             Command::SetExceptionBreakpoints(_) => {
                 let rsp = req.success(ResponseBody::SetExceptionBreakpoints(
                     SetExceptionBreakpointsResponse {
@@ -313,7 +313,6 @@ fn run_server<R: Read, W: Write>(
             }
             Command::Scopes(_) => {
                 let rsp = req.success(ResponseBody::Scopes(ScopesResponse {
-                    // TODO: Understand vectors
                     scopes: vec![],
                 }));
                 server.respond(rsp)?;

interp/src/debugger/cidr.rs

@@ -101,10 +101,6 @@ impl Debugger {
         let mut ctx = ir::from_ast::ast_to_ir(ws)?;
         let pm = PassManager::default_passes()?;
 
-        // Metadata stuff
-        let metadata = ctx.metadata.clone().unwrap();
-        let mapping = parse_metadata(&metadata).unwrap();
-
         // if !opts.skip_verification
         pm.execute_plan(&mut ctx, &["validate".to_string()], &[], false)?;
 
@@ -131,16 +127,24 @@ impl Debugger {
             &config,
         )?;
 
-        Debugger::new(&components, main_component, None, env, Some(mapping))
+        // Make NewSourceMap, if we can't then we explode
+        let mapping = ctx
+            .metadata
+            .map(|metadata| parse_metadata(&metadata))
+            .unwrap_or_else(|| Err(InterpreterError::MissingMetaData.into()))?;
+
+        Ok((
+            Debugger::new(&components, main_component, None, env).unwrap(),
+            mapping,
+        ))
     }
 
     pub fn new(
         context: &iir::ComponentCtx,
         main_component: &Rc<iir::Component>,
         source_map: Option<SourceMap>,
         env: InterpreterState,
-        metadata: Option<NewSourceMap>,
-    ) -> InterpreterResult<(Self, NewSourceMap)> {
+    ) -> InterpreterResult<Self> {
         let qin = ComponentQualifiedInstanceName::new_single(
             main_component,
             main_component.name,
@@ -151,23 +155,16 @@ impl Debugger {
 
         component_interpreter.converge()?;
 
-        Ok((
-            Self {
-                _context: Rc::clone(context),
-                main_component: Rc::clone(main_component),
-                debugging_ctx: DebuggingContext::new(
-                    context,
-                    &main_component.name,
-                ),
-                source_map,
-                interpreter: component_interpreter,
-            },
-            metadata.unwrap(),
-        ))
+        Ok(Self {
+            _context: Rc::clone(context),
+            main_component: Rc::clone(main_component),
+            debugging_ctx: DebuggingContext::new(context, &main_component.name),
+            source_map,
+            interpreter: component_interpreter,
+        })
     }
 
-    // probably want a different return type
-    // Return InterpreterResult of Program Status, new struct
+    // Go to next step
     pub fn step(&mut self, n: u64) -> InterpreterResult<ProgramStatus> {
         for _ in 0..n {
             self.interpreter.step()?;
@@ -181,10 +178,6 @@ impl Debugger {
         ))
     }
 
-    /// continue the execution until a breakpoint is hit, needs a different
-    /// return type or we need a different "update status" method to communicate
-    /// with the adapter. The latter might be more ergonomic, though potentially
-    /// less efficient
     pub fn cont(&mut self) -> InterpreterResult<()> {
         self.debugging_ctx
             .set_current_time(self.interpreter.currently_executing_group());

interp/src/debugger/new_parser.rs

@@ -24,17 +24,11 @@ impl MetadataParser {
             .map_err(|_| input.error("Expected number"))
     }
     fn group_name(input: Node) -> ParseResult<String> {
-        input
-            .as_str()
-            .parse::<String>()
-            .map_err(|_| input.error("Expected character"))
+        Ok(input.as_str().to_string())
     }
 
     fn path(input: Node) -> ParseResult<String> {
-        input
-            .as_str()
-            .parse::<String>()
-            .map_err(|_| input.error("Expected valid path"))
+        Ok(input.as_str().to_string())
     }
 
     fn entry(input: Node) -> ParseResult<(String, GroupContents)> {
@@ -59,10 +53,18 @@ pub fn parse_metadata(input_str: &str) -> InterpreterResult<NewSourceMap> {
     Ok(MetadataParser::metadata(input)?)
 }
 
+// Meta is expected as the following format, this is an example for reg_seq.futil
+
+// metadata #{
+//    wr_reg0: /path/to/file 10
+//    wr_reg1: /path/to/file 15
+//   }#
+
 #[cfg(test)]
 #[test]
 fn one_entry() {
     let entry = parse_metadata("hello: your/mom 5").unwrap();
+    dbg!(&entry);
     let tup = entry.lookup(String::from("hello"));
     assert_eq!(
         tup.unwrap().clone(),

interp/src/errors.rs

@@ -70,12 +70,16 @@ pub enum InterpreterError {
         #[from]
         pest_consume::Error<crate::debugger::source::metadata_parser::Rule>,
     ),
-    // New metadata
+    // Unable to parse metadata
     #[error(transparent)]
     NewMetadataParseError(
         #[from] pest_consume::Error<crate::debugger::new_parser::Rule>,
     ),
 
+    // Missing metadata
+    #[error("missing metadata")]
+    MissingMetaData,
+
     /// Wrapper for errors coming from the interactive CLI
     #[error(transparent)]
     ReadlineError(#[from] ReadlineError),

interp/src/main.rs

@@ -198,8 +198,7 @@ fn main() -> InterpreterResult<()> {
             } else {
                 None
             };
-            let (cidb, _) =
-                Debugger::new(&components, main_component, map, env, None)?;
+            let cidb = Debugger::new(&components, main_component, map, env)?;
             cidb.main_loop()
         }
         Command::Flat(_) => {