-
Notifications
You must be signed in to change notification settings - Fork 270
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
recursive alias causes error #1647
Comments
I'm not quite going to mark this WONTFIX, but the behavior is unlikely to change here. Apparently we expand aliases recursively (unlike bash); this error is not the nicest for if that forms a loop, but it does explain the issue. :) |
I remember seeing this bug reported a couple of months ago. Back then, I thought about writing a
I tried to check old issues/PRs/discussions (on Discord), read the POSIX spec, test it on bash, write test cases, decide the expected behavior, think about the potential change in the behavior, etc. Things got messy really fast. I think there are 2 ways to solve it:
@d0sboots What do you think about [2]? |
2 is basically what we already have, just with a bad error message. XD If you want to catch the exception and try to detect if it was a stack overflow, go ahead. |
I don't think we have a working detection of infinite recursion. In |
Maybe you misunderstood me. All known JS engines have working detection of infinite recursion. You can see that detection functioning in the bug report here, via the pasted exception. All that remains is to show a better message for this case. |
Oh, I got it now. I thought about implementing a custom "detector" for our alias/global alias, but catching the JS error is a good idea. |
I know nothing about POSIX compliance...but why not simply add the recursive alias info into the
|
To the second part, because the recursion can happen through more than one alias. |
You'd keep track of every rule already applied and don't apply a rule twice. Also implementing the posix rule that only unquoted words are potentially replaced by an alias could help here, since this would also allow writing an alias in the style TO tried to do. I doubt js really has an infinite recursion detection algorithm, since that would solve the halting problem. The most it probably does is to stop when the function call stack overflows. That's why TO experience a slight lag before the error was thrown. |
I have fixed a small flaw in the recursive check for alias application. However, I have not modified the function to ensure only unquoted commands are aliased, as I am still reviewing the POSIX quoting rules and how they are interpreted by the game's terminal. Additionally, I recommend not attempting to catch the "too much recursion" exception. This error varies by browser, leading to potential inconsistencies (more details here: MDN - InternalError: too much recursion). The function Please review my change and confirm if the new recursion check aligns with your expectations. |
This would prevent an alias definition like |
It might make sense for an alias only be able to call a 'base' command. |
The recursion is fine now; I fixed a small flaw in #1649. However, I noticed another issue just before merging, but I didn’t have time to address it last month. I'll have some free time at the start of next week and can finish it then. Apologies for the delay. |
Is there a reason to allow recursion by default? Why not add a flag to alias to allow processing recursion and don't do recursion otherwise....I could see how aliasing a command to a command with a specific option would be useful. |
version of the game: Bitburner v2.6.2 (633da38)
I have aliased
buy="buy -l"
. Now, when I type "buy" into the terminal and press enter, the game hangs briefly, then gives the following error:bitburnerSave_1725813539_BN1x1.json.gz
The text was updated successfully, but these errors were encountered: