-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: Add .dryrun Command Support #191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a couple of comments.
I would re-iterate to reduce overall complexity. A lot of stuff to simplify.
Good job
| `.exit` | This command will exit you console, but you can also do `Ctrl-C` or `Ctrl-D` | | ||
|
||
## License | ||
|
||
The ao and aos codebases are offered under the BSL 1.1 license for the duration | ||
of the testnet period. After the testnet phase is over, the code will be made | ||
available under either a new | ||
[evolutionary forking](https://arweave.medium.com/arweave-is-an-evolutionary-protocol-e072f5e69eaa) | ||
[evolutionary forking](https://arweave.medium.com/arweave-is-an-evolutionary-protocol-e072f5e69eaa)s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This s
seem to be a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, that's a typo.
await processInput(input, pid, data, owner, id, anchor); | ||
} | ||
|
||
async function extractTags(input, pid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code needs to be simplified. I suggest you reduce complexity by:
- Removing side effects throwable stuff to the top
- Generalize the parsing so it cant throw and then put it into a functor
- Try to reduce conditions to lower complexity
This should give an indication:
async function extractTags(input, pid) {
// Directly throw an error if input doesn't include "{", avoiding multiple checks.
if (!input.includes("{")) throw new Error("Tags not specified");
// Use optional chaining to simplify error handling.
const content = input.match(/\{(.*?)\}/)?.[1];
if (!content) throw new Error("Invalid Syntax Usage");
// Streamline tag parsing and creation with map (the functor), removing redundant else block.
const tags = content.split(",").map(pair => {
if (pair.includes("=")) {
let [key, value] = pair.split("=").map(item => item.trim().replace(/^['"]|['"]$/g, ""));
// Simplify replacement for "ao.id" with direct approach.
value = value === "ao.id" ? pid : value;
return { name: key, value };
} else {
// Handle action tags directly within the map function.
const action = pair.trim().replace(/^'|'$/g, "");
return { name: "Action", value: action };
}
});
// Check and add "Target"
if (!tags.some(tag => tag.name === "Target")) {
tags.unshift({ name: "Target", value: pid });
}
return tags;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P.S if target can be appended you can get rid of else al in all
if (!input.includes("{")) throw new Error("Tags not specified");
const content = input.match(/\{(.*?)\}/)?.[1];
if (!content) throw new Error("Invalid Syntax Usage");
return content.split(",").map(pair => {
const [key, rawValue] = pair.includes("=") ? pair.split("=") : ["Action", pair];
const value = rawValue.trim().replace(/^['"](.*)['"]$/, "$1").replace(/^ao\.id$/, pid);
return { name: key.trim(), value };
}).concat({ name: "Target", value: pid });
} | ||
|
||
|
||
const getCustomProcessId = (input, pid) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly cleaner.
const getCustomProcessId = (input, pid) => {
const match = input.match(/-p=(["'])(.*?)\1/);
if (!match) return null;
const customProcessId = match[2] === "ao.id" ? pid : match[2];
if (customProcessId.length !== 43) throw new Error("Invalid Process Id");
return customProcessId;
};
const result = await dryrun(dryrunParams); | ||
return result; | ||
} catch (error) { | ||
console.error(chalk.red("Error Performing DryRun:", error.message)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be right to propagate the error up in context? Why swallow it here.
🤔 I think this can be a simpler fix, if you use aos to start a process and you do not have @allquantor @mayurmarvel - What do you think? For example, if I call |
Description
This pull request addresses issue #156 by introducing support for the
.dryrun
command in theaos
project. Below is a summary of the changes made:Files Modified / Created:
/src/index.js
: Added support for.dryrun
command./src/commands/dryrun.js
: Implemented the functionality for the.dryrun
command./src/commands/dryrun.md
: Created a usage guide for the.dryrun
command.README.md
: Updated commands list to include.dryrun
./src/services/help.js
: Added.dryrun
to the help list.For the full usage guide and detailed changes, please refer to
/src/commands/dryrun.md
.Command Example
Flags List:
-p
: Specifies Process ID.-d
: Specifies data.-r
: Accesses the Result Object.-v
: Views the DryRun Parameters.Functions:
dryrun
: Executes a dry run to query Processes without sending a message.Testing
Performed manual testing to ensure the functionality of the
.dryrun
command in various scenarios.Features That Can Be Added
Related Issue
Closes #156