-
Notifications
You must be signed in to change notification settings - Fork 124
Do not display an error when an action didn't run #2905
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
Conversation
Signed-off-by: Seb Julliand <[email protected]>
|
👋 A new build is available for this PR based on 82d6c1f. |
chrjorgensen
left a comment
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.
@sebjulliand Did some testing of this PR - and your change seems to work.
However, I got some errors during testing:
- I ran an action with one command
CRTCMDon a.cmdstreamfile - no error, it compiled fine. - I duplicated the action above and added a
CPYcommand in front, so it hadCPYandCRTCMD, and ran this action. Now it failed with the following errors:
and the terminal showed this:
Fetching errors for CJORGENS1/'.
Error: Failed fetching table: CPD0012: Characters in qualifier beginning 'VSCODETEMP' not valid.
CPD0014: A matching apostrophe not found.
CPD0013: A matching parenthesis not found.
CPF0001: Error found on *N command.
This must an old error, because it also occurs running on code from the master branch.
Maybe the *EVENTF file retrieve code failing to generate the correct object name?
What is a |
|
@sebjulliand I think my comment above is an error in the old code - especially after working on PR #2926. You should probably just ignore that... |
Signed-off-by: Seb Julliand <[email protected]>
|
You're right my friend. I meant to write |
I'll check it out since I think I had the same error lately. Better fix it here while I'm at it. At least I'll fix the message that shows [Object object] |
Signed-off-by: Seb Julliand <[email protected]>
|
Back at you @chrjorgensen ! |
chrjorgensen
left a comment
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.
@sebjulliand Works as expected. 👍
Two remarks for some future PR's:
- If running an action with prompted command on multiple targets, the command is only prompted once. I would have thought it would be prompted for each target. Isn't that more logical?
- When the command is prompted, the variables are not replaced in the prompt, e.g.
&NAMEis still there instead of the name of the file.
These issues are in the master branch and need a new PR.
Many thanks @chrjorgensen for your review.
No, this was done on purpose, to prompt once and run many times. It felt wrong to show the prompt for each target.
Which is related to the point above. It should be replaced when running on a single target though. |
Changes
This PR prevents the error notification from showing up when a Custom action has a prompt (either a custom prompt or a
?prompt) and that prompt is closed or cancelled.How to test this PR
Run the test on a single item and on multiple items too
Checklist