rust-osdev / acpi

Rust library for parsing ACPI tables and interpreting AML
Apache License 2.0
203 stars 65 forks source link

Make `aml_tester` more ergonomic to use #142

Open IsaacWoods opened 1 year ago

IsaacWoods commented 1 year ago

At the moment, aml_tester works, but is pretty janky to use. Some improvements would be:

rw-vanc commented 1 year ago

What are your feelings on clap 4? Would you prefer derive or just update the clap code that's there? My suggestion is to allow files specified by --file or as positionals. --file can have multiple occurrences and be mixed with --path to specify order of parsing. Positionals would be parsed in the order given. --reset would cause namespace to be per table, no --reset would cause all files to be parsed into the same table.

rw-vanc commented 1 year ago

It's easier to implement mixing of files and directories if they are the same arg name, or if they are just all positionals. What about dropping --files and --path, and just taking both file and path arguments as positionals? --path could be kept for backwards compatibility.

IsaacWoods commented 1 year ago

No issues with updating clap, so have merged #149 - no strong feelings on derive vs not either, whatever you find most ergonomic.

I think it would be nicer to maintain two modes: one for parsing a directory of files independent of each other (the current setting), and another for parsing a set of tables in order in the same namespace. Can we pass an option (e.g. --parse_table_series or whatever) then a list of positional paths in the order of parsing to allow for this separate mode? I'm not too fussed which we make the default, as long as it's documented and both are possible explicitly.

Also no need generally to keep things backwards compatible with aml_tester, as it's entirely an internal testing tool atm afaik.

rw-vanc commented 1 year ago

I have an implementation of using positionals. You can use --path DIR or positional FILE ..., but not both. I did not implement mixing files and dirs as positional, but I can if that's useful. File order is significant, aml files produced by compiling are parsed in the same order as their asl files in the original command line. Duplicates are removed, so the first occurrence of a file is the order used. There is a --reset option to create a fresh context after each table, the default is to reuse the context. I'm just doing some testing and will submit today.

Usage: aml_tester [OPTIONS] <--path <DIR>|FILE.{asl,aml}>

Arguments:
  [FILE.{asl,aml}]...  

Options:
      --no-compile  Don't compile asl to aml
      --reset       Clear namespace after each file
  -p, --path <DIR>  
  -h, --help        Print help
  -V, --version     Print version