Skip to content

quitcd: fix old bug and feat. for modular export for nushell#1806

Merged
jarun merged 1 commit intojarun:masterfrom
BeyondMagic:master
Feb 12, 2024
Merged

quitcd: fix old bug and feat. for modular export for nushell#1806
jarun merged 1 commit intojarun:masterfrom
BeyondMagic:master

Conversation

@BeyondMagic
Copy link
Contributor

@BeyondMagic BeyondMagic commented Feb 1, 2024

  • now users can import as a module instead of just copying and paste with use nnn.nu
  • also refactor everything with internal commands, and add commentary, and remove unnecessary --force in rm.
  • remove having to import nnn environment variable NNN_TMPFILE, now it only launches for the command.

image

@N-R-K
Copy link
Collaborator

N-R-K commented Feb 2, 2024

instead of just copying and paste

There was no need for copy paste. You can just source the file (I'm assuming nu shell has source).

@BeyondMagic
Copy link
Contributor Author

There was no need for copy paste. You can just source the file (I'm assuming nu shell has source).

Nushell aims to focus on modularity, by replacing source alike-files with modules we can hide and use whenever we want.

@BeyondMagic BeyondMagic requested a review from N-R-K February 2, 2024 14:30
@N-R-K
Copy link
Collaborator

N-R-K commented Feb 2, 2024

with modules we can hide and use whenever we want.

Not sure if this makes sense or not since this isn't some "library" meant to be used wherever. But then again, I don't use nushell, so I could be wrong.


@J-Kappes @mmai could you guys review and/or test this pull request?

@BeyondMagic
Copy link
Contributor Author

with modules we can hide and use whenever we want.

Not sure if this makes sense or not since this isn't some "library" meant to be used wherever.

You can start by reading about modules here.

And you can test by downloading nushell and then opening the terminal, typing out the following commands step by step:

  1. nu
  2. use <this repo>/misc/quitcd/quitcd.nu
  3. n

Then go to another directory and exit. There we go.

Think of this by also enabling users to build upon this simple function.

@jarun
Copy link
Owner

jarun commented Feb 2, 2024

@N-R-K is this good to go?

@N-R-K
Copy link
Collaborator

N-R-K commented Feb 3, 2024

@N-R-K is this good to go?

Waiting for other nushell users to test/review.

And you can test by downloading nushell [...]

Sorry, not interested in downloading/running pre-built binaries and also not interested in compiling thousands of dependencies just to test something I have no intention to use.

@jarun
Copy link
Owner

jarun commented Feb 8, 2024

@BeyondMagic please squash to a single commit.

@mmai can you please review?

@BeyondMagic
Copy link
Contributor Author

@jarun done.

Copy link
Contributor

@mmai mmai left a comment

Choose a reason for hiding this comment

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

Hello @BeyondMagic . Thank you for the modular export. I saw two problems with the config_home setting and the temp file removal.

@BeyondMagic BeyondMagic requested a review from mmai February 10, 2024 18:36
Copy link
Collaborator

@N-R-K N-R-K left a comment

Choose a reason for hiding this comment

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

This looks better. One small nit on the wording of --selective flag.

@BeyondMagic
Copy link
Contributor Author

This looks better. One small nit on the wording of --selective flag.

Fixed, had a few problems pulling from the main branch, but now the log history is fine.

@jarun jarun merged commit 0f62c62 into jarun:master Feb 12, 2024
@jarun
Copy link
Owner

jarun commented Feb 12, 2024

Thanks guys!

@jarun jarun mentioned this pull request Feb 12, 2024
10 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants