Skip to content
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

script autocomplete only works in really simple cases #522

Open
aalku opened this issue May 22, 2023 · 12 comments
Open

script autocomplete only works in really simple cases #522

aalku opened this issue May 22, 2023 · 12 comments
Labels
enhancement New feature or request

Comments

@aalku
Copy link

aalku commented May 22, 2023

  • version Bitburner v2.2.2 (d3f9554)

Hi! It seems script autocomplete only works with a simple list, not depending on which argument you are typing and what are the previous ones because the script can't really tell which one you are typing.

If you type "./script.js arg0" (no space before the tab) args will be ["arg0"] but if you type "./script.js arg0 " (space before the tab) then args will be ["arg0"] too but the user is clearly trying to do two different things. In the first one is trying to autocomplete arg0 with arg0.js for example and the second one is asking for the second argument.

I think the solution might be either telling which number of argument the user is trying to complete or encoding it in args[] adding a trailing "" if the user pressed tab after a space so there is one more argument but still blank.

The problem is not only after space because the script can't tell if it's after a space or not.

@CrafterKolyan
Copy link
Contributor

Completely agree with this one. Also had a problem with autocomplete function. Possible solution can be that args that are passed to autocomplete functions should always contain prefix of currently typed argument. Here are my examples


Terminal:

./foo.js |

, here | means where the cursor currently is
Arguments:

[""]

Terminal:

./foo.js n00|

Arguments:

["n00"]

Terminal:

./foo.js n00dles|

Arguments;

["n00dles"]

Terminal:

./foo.js n00dles |

Arguments:

["n00dles", ""]

@ABruel
Copy link

ABruel commented Jun 8, 2023

I agree an empty string would be nice, however.

The argument the user is typing is either the last in the array, or the next one. All you got to do to differentiate between the two is check whether or not the last argument is already complete.

In your example of ./script.js arg0 you need to determine if the regex arg0.+ is a valid completion, ie you'd check if the scripts array has a script starting with arg0. if it doesn't, then either the user typed an invalid argument or arg0 is already a complete argument and the user is already typing the next one.

You can take a look on this gist for a better idea. I don't think there's any bugs in it, at least I haven't found any.

I say that i agree with the idea because this code is a bit annoying to write, but I do believe you can work around the limitations of the current system.

@aalku
Copy link
Author

aalku commented Jun 8, 2023

Your idea is awesome but I think there's no reason to make us do that. 😅

It's like "Your car doesn't need a gas deposit, it can have a tube to feed the engine with a bottle from the driver's seat".

Yes you can, and it's a great idea to overcome the problem if you need it, but you shouldn't need it in my opinion. So the bug report.

@ABruel
Copy link

ABruel commented Jun 8, 2023

This isn't a bug. It's intended behavior. What you're asking for is a feature. A feature that probably won't be implemented because

  1. It's a breaking feature, meaning thousands of autompletes out there would need to be rewritten from scratch. It's even a double jeopardy, since a simple eslint analysis won't be able to detect the error.

  2. Can be circumvented with only a few extra lines of code. Not only that, but extremely repeatable code, which means it can be abstracted to a function.

@d0sboots
Copy link
Collaborator

d0sboots commented Jun 8, 2023

This isn't a bug. It's intended behavior. What you're asking for is a feature.

The issue tracker is for both bugs and feature requests; the line between the two is not always clear cut and it's generally useful to track them the same way.

@aalku
Copy link
Author

aalku commented Jun 8, 2023

This isn't a bug. It's intended behavior. What you're asking for is a feature. A feature that probably won't be implemented because

  1. It's a breaking feature, meaning thousands of autompletes out there would need to be rewritten from scratch. It's even a double jeopardy, since a simple eslint analysis won't be able to detect the error.

  2. Can be circumvented with only a few extra lines of code. Not only that, but extremely repeatable code, which means it can be abstracted to a function.

It is broken as it is because it gives you confusing information. I'm sure many scripts assume you press tab before typing anything and many scripts assume the other thing, and use the length of the array one way or the other and can't be well used the other way.

The way to fix it with full reverse compatibility is to introduce a new parameter that you can use if you want, indicating the index of the argument to complete.

If you don't have it declared or you don't use it then everything remains the same, but you can declare it and use it if you want.

@CrafterKolyan
Copy link
Contributor

CrafterKolyan commented Jun 8, 2023

@ABruel The problem is that if there are 2 possible completions and one is the prefix of another then it's impossible to understand if the user has typed only half or finished typing.
Example: two possible completions for first argument:

["run", "run-all"]

User typed in "run" and you get in autocomplete function:

["run"]

and now you don't know if it's the end of the previous argument or the start of the second.

This can be an issue when you try to create a script which works with single files and directories for example.

@ABruel
Copy link

ABruel commented Jun 8, 2023

@CrafterKolyan you are correct, that does break things.

The issue tracker is for both bugs and feature requests; the line between the two is not always clear cut and it's generally useful to track them the same way.

I know that, I was just correcting that this is not a bug as previously stated. What I said after it doesn't really change...

So maybe even that isn't true anymore.

Now, this issue did get me thinking...

@aalku, yes, it would work with an extra argument, at most it would break declarations with ...args that don't use fixed indexes, but I really don't see a reason anyone would have their autocomplete set up like that.

Now, in case this is not implemented, there's still one simple way to actually do the add empty string to the end if it ends with space idea (maybe even the caret position one, but I haven't tested it):

export function autocomplete(data: AutocompleteData, args: string[]) {
  // We are typing, so input definitely exists, and is an input element.
  const fullTerminalValue: string = (
    document.getElementById("terminal-input")! as HTMLInputElement
  ).value
    /**
     * trim start
     * for some reason the terminal input has a space at the start.
     * maybe it should be on the host element to the side?
     */
    .trimStart()
    .split(" ")
    /**
     * remove the first two elements since they should be "run" and the script name
     *
     * I tried using ./scriptname but it doesn't seem to call autocomplete,
     * is this intended?
     */
    .slice(2)
    .join(" ");

  console.log('FULL ARGS VALUE: "' + JSON.stringify(fullTerminalValue) + '"');

  return [];
}

Here's the result.
image

@ABruel
Copy link

ABruel commented Jun 8, 2023

While looking through the code trying to find if './<script>' isn't supposed to call autocomplete, I actually found:

[autocomplete] is not officially supported yet and the API might change. It is also only supported in ns2

So I guess this could even be implemented as a breaking change.

Oh well, I found a way to actually to this, so I'm happy either way. I will however re-implement my autocompletes assuming the last argument will be an empty string if it ends with a space. If it does come to be natively supported, I'll just change the function to return args instead of grabbing the data from the terminal input.

@aalku
Copy link
Author

aalku commented Jun 8, 2023

I think ./script autocomplete did work on version 2.2.x and broke in 2.3.x.

@cactusdualcore
Copy link

cactusdualcore commented Jan 29, 2024

Arrays can have named attributes (think length) and normal code can set those on any array. It feels like just adding a incomplete attribute to the args array would give everybody the information needed in a non-breaking way?

@s1495k043
Copy link

s1495k043 commented Jun 29, 2024

I write a method for this.

function autoCompleteItem(args: string[], data: { [key: string]: string[] }) {
  //last input args
  const last = args.slice(-1) || "";
  //if match return data[last] else return prefix
  return data[last] || Object.keys(data);
}

just use like this

//this is parameter for upgrade server memory
export function autocomplete(data: AutocompleteData, args: string[]): string[] {
  return autoCompleteItem(args, {
    "--host": data.servers,
    "--ram": [8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096],
  });
}

If last input match key then return match array, else return all key

image
image

-t seems to be a reserved word, do not use it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants