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

feat: rewrite in nushell #52

Open
wants to merge 46 commits into
base: master
Choose a base branch
from
Open

feat: rewrite in nushell #52

wants to merge 46 commits into from

Conversation

Elsie19
Copy link
Member

@Elsie19 Elsie19 commented Dec 5, 2024

We still need a way to add i18n. Done.

@Elsie19 Elsie19 linked an issue Jan 1, 2025 that may be closed by this pull request
@ajstrongdev
Copy link
Member

Issues to note whilst testing:

  • Seems to hang on snapd, when ^C is pressed this is the output:
^CError: nu::shell::column_not_found

  × Cannot find column 'Publisher'
    ╭─[/usr/share/rhino-pkg/modules/pluggables/snap.nu:19:15]
 18 │             | detect columns --guess
 19 │             | reject Publisher Version
    ·               ───┬── ────┬────
    ·                  │       ╰── cannot find column 'Publisher'
    ·                  ╰── value originates here
 20 │             | rename --column { Name: pkg }
    ╰────
  • Requires support for nix and am package managers, as these can be enabled during rhino-setup

@Elsie19
Copy link
Member Author

Elsie19 commented Jan 1, 2025

What did you search..? I need reproducible input.

@ajstrongdev
Copy link
Member

What did you search..? I need reproducible input.

rpk search discord

@Elsie19
Copy link
Member Author

Elsie19 commented Jan 2, 2025

Wait you said this was in a docker container?

@Elsie19
Copy link
Member Author

Elsie19 commented Jan 3, 2025

We determined the issue only happens in a docker container.

@Elsie19 Elsie19 marked this pull request as ready for review January 3, 2025 14:49
@Elsie19
Copy link
Member Author

Elsie19 commented Jan 3, 2025

Testing

Install nutext

Nutext is the nu plugin I made that enables us to bring gettext functionality to nu programs.

To install it, first clone the repo:

git clone https://github.com/Elsie19/nu_plugin_nutext

cd into it and run:

make build
sudo make install

Installing rpk2

rpk2 is the actual program.

To install it, first clone the repo:

git clone -b nushell https://github.com/rhino-linux/rhino-pkg

cd into it and run:

sudo make install

And you should be all good for testing!

Notes

  1. Do not run in a docker container if you are testing snap functionality or if snap is installed in it at all, because it will hang

Reverting/uninstalling

Uninstalling

Nutext

To uninstall nutext, go back into the cloned nu_plugin_nutext project and run:

sudo make uninstall

rpk2

This is harder because it likely overwrote your bash rpk, and I haven't tested exactly, but reinstalling rpk should do the trick:

pacstall -I rhino-pkg-git
sudo rm -rf /usr/share/rhino-pkg/

@Elsie19 Elsie19 added the enhancement New feature or request label Jan 3, 2025
@Elsie19
Copy link
Member Author

Elsie19 commented Jan 3, 2025

Other stuff

We will have to keep nushell-bin, rhino-pkg-git, and nutext-bin tightly coupled, moreso nutext-bin and nushell-bin, as we will need to update nutext-bin for every nushell update.

Copy link
Member

@ajstrongdev ajstrongdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One issue is that the program can't detect the nutext plugin, causing an exception.
image

@Elsie19
Copy link
Member Author

Elsie19 commented Jan 5, 2025

Apparently you need to add the plugin in a nu shell before being able to run it. I will look more into it.

@Elsie19 Elsie19 requested a review from ajstrongdev January 5, 2025 18:52
Copy link
Member

@ajstrongdev ajstrongdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few issues:

  • Unexpected behaviors when not giving a number as an input.
  • No rpk help command. It seems like you copy/pasted rpk help output to rpk -h, which presents issues. Furthermore it seems like auto-generated help information displays beneath this
  • Using apt-remove instead of apt autoremove
  • Running rpk without arguments returns an error, when it should return the content of the help command.

What i mean by the first point:

  • When running rpk install discord and I select no options, it will say "selecting discord from snap" and then... fail?
  • When running rpk remove discord and I press enter without putting a number in, it throws an error.

Please ensure that rpk2 has consistent functionality with the first rpk.

@Elsie19
Copy link
Member Author

Elsie19 commented Jan 6, 2025

  • Furthermore it seems like auto-generated help information displays beneath this

That cannot be turned off btw.

@Elsie19
Copy link
Member Author

Elsie19 commented Jan 6, 2025

A few issues:

  • Unexpected behaviors when not giving a number as an input.
  • No rpk help command. It seems like you copy/pasted rpk help output to rpk -h, which presents issues. Furthermore it seems like auto-generated help information displays beneath this

There is no rpk help command. You are confusing this behavior with the catch-all flag:

rhino-pkg/rhino-pkg

Lines 200 to 203 in 0b7fc65

*)
echo "$help_flag"
exit 1
;;

  • Using apt-remove instead of apt autoremove

That's not a command, unless you meant apt auto-remove:

sudo apt --fix-broken install && sudo apt auto-remove -y

In which case those are the same in apt.

  • Running rpk without arguments returns an error, when it should return the content of the help command.

What i mean by the first point:

  • When running rpk install discord and I select no options, it will say "selecting discord from snap" and then... fail?
  • When running rpk remove discord and I press enter without putting a number in, it throws an error.

Please ensure that rpk2 has consistent functionality with the first rpk.

Ok.

@Elsie19 Elsie19 requested a review from ajstrongdev January 9, 2025 04:11
@oklopfer
Copy link
Member

oklopfer commented Jan 9, 2025

  • Using apt-remove instead of apt autoremove

That's not a command, unless you meant apt auto-remove:

sudo apt --fix-broken install && sudo apt auto-remove -y

Took you 2 months to figure out apt-remove isn't a command, hope it doesn't take as long to figure out that you replied to AJ's typo with a link to the old bash code that you typoed in the first place:

^sudo apt-get apt-remove

Don't keep asking me to test the code if you haven't done bare minimum testing yourself. rpk cleanup is all you had to run. And given the link above, it is evident you haven't tested this basic functionality since at least before you even opened this PR.

| insert provider 'apt'
}
} else {
[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Status: Rollover
Development

Successfully merging this pull request may close these issues.

BUG: Can't install snap package with --classic rework: switch to rhinu-pkg
3 participants