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(plugin-patch): Expose Temp Patch Path #4831

Closed
wants to merge 3 commits into from

Conversation

therynamo
Copy link

@therynamo therynamo commented Sep 7, 2022

What's the problem this PR addresses?

This PR aims to improve the ability to write plugins that pertain to "patching" with Yarn.

In the Clipanion docs there is a section about how to add options to an existing command. In order to do this, as you may know, you need access to the original Command and then you can extend from that.

Yarn berry does not expose either command in its plugin-patch package - which forced me to have to copy/paste them into my codebase (after a few failed attempts to work around it).

Since the expected practice is to extend the command you're plugging - then I think its reasonable to export these command so they may be extended.


This PR also includes another change - related to patch plug-ability. The temp folder path is not stored anywhere accessible from within the command. This makes sense - as it was never exposed and if it was needed it could be quickly modified within the existing file.

With the exposure of these classes - its also helpful to have to path to which the patch was written to. An example that may help is - the plugin I'm writing will automatically open your "preferred editor" if set (much like git does - see core.editor).

...

How did you fix it?

...

  • export both commands in this package
  • added a variable to the patch class to expose to extended commands

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz self-requested a review September 7, 2022 20:15
@RDIL RDIL changed the title feat: Expose Commands and Add Temp Path feat(plugin-patch): Expose Commands and Add Temp Path Sep 7, 2022
@arcanis
Copy link
Member

arcanis commented Sep 15, 2022

Thanks! In the end it got superseded by #4834, which exports commands from all plugins

@arcanis arcanis closed this Sep 15, 2022
@therynamo
Copy link
Author

therynamo commented Sep 15, 2022

@arcanis - thanks for the heads up. I am glad the exports are there.

Could we re-open this and limit the change-scope to the tmp path then? I also think it would still be highly valuable to expose the temp directory path for the patch.

https://github.com/yarnpkg/berry/pull/4831/files#diff-9ce542250c8e60caab9029c5a233f6a34e783e82de5f1b995ec7463673f532eeR81 - specifically this change.

@therynamo therynamo changed the title feat(plugin-patch): Expose Commands and Add Temp Path feat(plugin-patch): Add Temp Path Sep 15, 2022
@therynamo therynamo changed the title feat(plugin-patch): Add Temp Path feat(plugin-patch): Expose Temp Patch Path Sep 15, 2022
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