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

TERMINAL: Added grep #1343

Closed
wants to merge 61 commits into from
Closed

Conversation

muesli4brekkies
Copy link
Contributor

@muesli4brekkies muesli4brekkies commented Jun 3, 2024

A simple implementation of grep.

There's a few ideas I've thought to add, to make it more grep-like, such as;

  • regexp support (should be pretty simple),
  • -n to disable line-numbers,
  • -B/A/C n to provide context,

But I thought I'd make the PR and get the ball rolling.

:)

@d0sboots
Copy link
Collaborator

d0sboots commented Jun 3, 2024

Nice!
My main feedback would be, I'd suggest to support regex now, and try to make the output match "real" grep as much as possible. IMO it's more important for the existing features to work correctly, than to support lots of optional flags (possibly slightly weirdly.)

@muesli4brekkies
Copy link
Contributor Author

muesli4brekkies commented Jun 3, 2024

Don't you know, I'm making my own POSIX standard, in BitBurner! BOSIX!

Nah you're absolutely right 😁 . I'll give those changes a bash ASAP.

What are your thoughts on the globbing behaviour with no file args passed in? Implementing /* syntax should be simple enough,, but is it overkill for the game?

I think the default "search everything" is a useful feature, if not particularly compliant.

PS - are ANSI codes OK for the colours, or should I be using styles? All I need are the terminal primaries anyway.

@d0sboots
Copy link
Collaborator

d0sboots commented Jun 3, 2024

What are your thoughts on the globbing behaviour with no file args passed in? Implementing /* syntax should be simple enough,, but is it overkill for the game?

Hmmm that's a good question. Although it would be technically possible to still use stdin, that's not really a thing with our shell, and we don't have pipelines or redirections. So defaulting to ./* probably makes the most sense.

PS - are ANSI codes OK for the colours, or should I be using styles? All I need are the terminal primaries anyway.

ANSI codes are fine, as long as the layer you're outputting to renders them :D

@muesli4brekkies
Copy link
Contributor Author

I'm gonna run out of option flags to implement soon 😄

@G4mingJon4s
Copy link
Contributor

About that formatting of the arguments, like when you would do grep --help in linux. Do we stick to this [OPTION] formatting or do we try to match it to existing commands in the game? I'm asking because I want to "copy" your approach for rm to make it more uniform.

Also, I see you parse the arguments yourself. We have a library import libarg from "arg" for that, so I'm wondering if you have a reason to not use it?

@muesli4brekkies
Copy link
Contributor Author

muesli4brekkies commented Jun 8, 2024

About that formatting of the arguments, like when you would do grep --help in linux. Do we stick to this [OPTION] formatting or do we try to match it to existing commands in the game?

I figured replicating the command as close as I reasonably could would be ideal and potentially useful, but also most fun and satisfying!

And I think the other commands should be adjusted to be as close as possible as well, for fun and profit. 😁

Also, I see you parse the arguments yourself. We have a library import libarg from "arg" for that, so I'm wondering if you have a reason to not use it?

I was totally unaware of ... from "arg";! Well hidden, lol - I did cursorily look around the repo for an arg lib, but when not immediately finding one I really didn't fancy pulling a new dep, so I started hacking.

Do you know if it's https://www.npmjs.com/package/jargs this? There's no documentation as to what it is (that I can find) but from the few (five) uses it has in the repo, it looks similar.

ps: I'm a little disappointed you pointed that out xD. I'm quite pleased with the shape I got my little parser into haha.

@G4mingJon4s
Copy link
Contributor

Do you know if it's https://www.npmjs.com/package/jargs this? There's no documentation as to what it is (that I can find) but from the few (five) uses it has in the repo, it looks similar.

Sadly, beyond the uses within the game, I have no idea what this package is or what it does.

I'm a little disappointed you pointed that out xD. I'm quite pleased with the shape I got my little parser into haha.

It's fine if you leave it as is. I'm changing some stuff from the rm command and am also not using the library. First, I wanted to, but it doesn't support single flags with multiple characters, e.g. -rf, as it's not the standard (I think?). If you don't want to use it, it's probably fine.

@muesli4brekkies
Copy link
Contributor Author

I have multiflags working now anyhow. 😃 .

While multiflag options might not be POSIX (assuming that they are not), they are definitely Linux. It's something I would miss.

@muesli4brekkies muesli4brekkies deleted the branch bitburner-official:dev June 8, 2024 21:04
@muesli4brekkies muesli4brekkies deleted the dev branch June 8, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants