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

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

Merged
merged 1 commit into from
Feb 12, 2024

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

misc/quitcd/quitcd.nu Outdated Show resolved Hide resolved
@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.

@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.

misc/quitcd/quitcd.nu Outdated Show resolved Hide resolved
misc/quitcd/quitcd.nu Outdated Show resolved Hide resolved
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.

misc/quitcd/quitcd.nu Outdated Show resolved Hide resolved
@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
7 checks passed
@jarun
Copy link
Owner

jarun commented Feb 12, 2024

Thanks guys!

@jarun jarun mentioned this pull request Feb 12, 2024
19 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants