Skip to content

Add reference to Nu Plugin Tracer in the Nushell Book. #1372

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

Merged
merged 4 commits into from
May 1, 2024

Conversation

tesujimath
Copy link
Contributor

No description provided.

@fdncred
Copy link
Contributor

fdncred commented Apr 30, 2024

I've been on the fence about this PR, here's why. We allow other 3rd party software to be mentioned in our docs, but this is not just an external tool but something that is tied to the deep "guts" of nushell plugins. So, it makes me feel responsible that it always works, even though it's not nushell owned software. I'm not trying to make a case to "own" your software. Just sharing my feelings to have a conversation. I think we can probably land this. 🤔

The last thing that bothers me about this tool is that there's no on/off switch. Correct me if I'm wrong, but once you "turn it on" with plugin add --shell, it will keep appending to the files in your home dir until "turn it off" with plugin rm. Is that correct? I was playing around with it and thought it was cool but then I was like, oh these files are getting larger and larger, how do I make it stop. 😆

If it's not already in our awesome_nu repo, it could definitely go there. I don't have the same feelings about that repo.

@tesujimath
Copy link
Contributor Author

I did think about adding it to awesome_nu, but I decided against it. Here's why.

The tracer is not a plugin per se. It's a tool for plugin developers. So it ought to be referenced from the place that plugin developers look to learn how to create plugins.

In contrast, the list of plugins on awesome_nu is a reference for end users.

Unless you envisage a new section, say Tools for Plugin Developers? But that would be a pretty small section so far! 😁

@fdncred
Copy link
Contributor

fdncred commented May 1, 2024

Unless you envisage a new section, say Tools for Plugin Developers?

I have no objections to another section.

Also, I ran into another problem. Since it's named in the same style as a plugin, it's easily mistaken for a plugin and fails to be registered with plugin add as in toolkit add plugins.

So, for me, having it named like a plugin isn't great. Also, as mentioned previously, not having some type of on/off switch makes it a little tricky to use too. There should at least be a warning that you'll fill up your file system if you don't disable it.

@tesujimath
Copy link
Contributor Author

Yes, I've also had reservations about the name. How about trace_nu_plugin?

I'll add a warning to the README about the ongoing accumulation of trace output. (I don't think it's worth adding an actual feature to mitigate that though, would make it unreasonably complicated.)

@fdncred
Copy link
Contributor

fdncred commented May 1, 2024

trace_nu_plugin

Yes, I think that's even more descriptive.

I'll add a warning to the README about the ongoing accumulation of trace output.

Sounds good. I'll land this PR with these changes.

@tesujimath
Copy link
Contributor Author

That's done. 😎

@fdncred
Copy link
Contributor

fdncred commented May 1, 2024

I thought you were going to put a warning here too?

@tesujimath
Copy link
Contributor Author

Ah sorry, I can also do that if you like.

Its also in the README for trace_nu_plugin
@tesujimath
Copy link
Contributor Author

How's that?

@fdncred
Copy link
Contributor

fdncred commented May 1, 2024

Looks good. Just one small change to help people know how to remove a plugin since it's new.

@fdncred fdncred merged commit 0ff1336 into nushell:main May 1, 2024
2 checks passed
@tesujimath
Copy link
Contributor Author

Thanks!

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.

2 participants