Skip to content

Rewrite of 'Add Folder To Project'. #8904

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 2 commits into from
Aug 8, 2024

Conversation

dennykorsukewitz
Copy link
Contributor

@dennykorsukewitz dennykorsukewitz commented Apr 4, 2024

  • I'm the package's author and/or maintainer.
  • I have have read the docs.
  • I have tagged a release with a semver version number.
  • My package repo has a description and a README describing what it's for and how to use it.
  • My package doesn't add context menu entries. *
  • My package doesn't add key bindings. **
  • Any commands are available via the command palette.
  • Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.
  • If my package is a syntax it doesn't also add a color scheme. ***
  • If my package is a syntax it is named after the language it supports (without suffixes like "syntax" or "highlighting").
  • I use .gitattributes to exclude files from the package: images, test files, sublime-project/workspace.

Hi guys,
my package is Sublime-AddFolderToProject

AddFolderToProject is a Sublime Text package that streamlines the process of adding and removing folders to and from your projects. It provides a set of commands and context menu options to manage your project's folder structure directly from the editor.

This extension already started as a small function in the Znuny-Sublime package.
However, I had to extend this function and place it in an extra package.

My package is similar to AddFolderToProject-SublimePlugin:
I noticed that there is already an AddFolderToProject-SublimePlugin package.

However it should still be added because:
But apparently this is no longer maintained since 09.02.2015. Issues and PullRequest were not further processed.

I have compiled and revised my functions and the functions of my colleague.
I have solved all known problems and now want to make it available.

Of course, I don't want to displace my colleague's work. That's why I've given the package the 2.
Or should we replace the existing one?

I will tag a release if nothing speaks against this merge from your side.

ChannelRepositoryTools: Test Default Channel

Running Default Channel Tests
  ----------------------------------------------------------------------
  Ran 9682 tests in 0.175s
  
  OK

@dennykorsukewitz dennykorsukewitz changed the title Added Add Folder To Project 2. Added 'Add Folder To Project 2'. Apr 4, 2024
@dennykorsukewitz dennykorsukewitz marked this pull request as ready for review April 4, 2024 22:08
@deathaxe
Copy link
Contributor

deathaxe commented Apr 7, 2024

If an unmaintained old package already exists with nearly the same (or less) functionality, a valid option was to replace it (or take over its name).

Another but way simpler package to note is AddRemoveFolder.

With all the functionality of your package however a name like "Folder Manager" was probably a better fit.

Actually I like the idea of having a single or limited amount of high quality packages, which handle all major aspects of managing folders and cooperate well with packages like Project Manager or Project and Workspace Management.

Other recently updated packages with focus to handling folders are

  • a Folder Aliaser to change folders' labels.
  • a AutoProjects to auto-generate sublime-project files for folders opened via subl <folder>.

Note

I was already created a command to remove folders, too, which makes use of ST's InputHandlers for a more seamless and more modern looking integration into Command Palette.

grafik


With out-of-the-box existing Project: Add Folder command, I wonder if it would also make sense to group commands of your package under Project: for usability-consistency reasons, e.g.:

  • Project: Add current file's folder
  • Project: Add current file's parent folder
    note: the default "Project: Add Folder" opens a dialog box to choose any folder
  • Project: Remove Folder
  • ...

A command Copy Path is already available via context menu in side bar. It copies a selected folder's or file's absolute path. So the commands in your package are redundant.

Packages like File Manager also provide more variants of them.


Any new plugin must opt-in to python 3.8 plugin_host via .python-version.

@dennykorsukewitz
Copy link
Contributor Author

Hey @deathaxe ,
thanks for all the feedback.
I've seen some of the other packages, but you've also shown me a few new ones.

If we replace the existing one, I would have to do the following:

  • .python-version.
  • change code to use of ST's InputHandlers
  • group commands under Project: for usability-consistency reasons
  • remove obsolete Copy Path command
  • release

But now I'm honestly confused as to what I should do best. What would be your suggestion?

Best regards

@braver
Copy link
Collaborator

braver commented Jun 19, 2024

Sorry for letting you dangle for a while there. Been busy with.. "life" I guess 😅 And I must say this thread had be a little bit confused as well.

Or should we replace the existing one?

I would say yes. "Add Folder To Project" is positively ancient, and we can see there are already various other approaches to covering parts (or more) of that feature set, a new replacement for that existing package sounds like a good move to me. We of course need to check with the authors of that package to see if they object, and if maybe you can simply contribute your work to that existing repo. And then make sure you message the user base about what happened.

I wonder if it would also make sense to group commands of your package under Project:

I can see how that would be nice, also because AddFolderToProject is a bit of a long name. But you could argue that the labelling is also there to indicate which package is providing the feature. So perhaps leave it as is (and consider a shorter name for your package 😉).

remove obsolete Copy Path command
.python-version.

Those sound like good (and easy) improvements.

change code to use of ST's InputHandlers

You could do that at a later point.

TLDR; first step would be to consider replacing, or contributing to, the original package by @DavidGerva

@braver braver added the stale The pull request needs to be updated but has not been within the recent past (2 weeks) label Jul 16, 2024
@braver
Copy link
Collaborator

braver commented Jul 16, 2024

Please respond to the feedback to continue the review.

@dennykorsukewitz
Copy link
Contributor Author

Hi, I'll give you more feedback tomorrow.

@dennykorsukewitz
Copy link
Contributor Author

Hi @braver

I wrote to David @DavidGerva again.

Hi David,

first of all thanks for your OpenSource work!
Especially in the SublimePlugin area.

https://github.com/DavidGerva/AddFolderToProject-SublimePlugin

I've been using your AddFolderToProject plugin for quite a long time, but I still had requests for improvements.
Unfortunately, you are no longer active in this repo and therefore current points were no longer processed. 

See PR https://github.com/DavidGerva/AddFolderToProject-SublimePlugin/pull/1

I have now incorporated my own extensions into AddFolderToProject https://github.com/wbond/package_control_channel/pull/8904 and revised and updated your existing functions. However, understandably there should not be a second AddFolderToProject plugin.

Now I'm standing around a bit. The plan would be to replace your plugin with mine.
The same functions as before, just a few more.

But we can't just do this without your permission. The great respect among us developers should be preserved.

Would you give us permission to do this? Or how do you feel about it?

Kind regards,
Denny

I hope we get an answer.
As soon as I have received feedback, I will let you know.

P.S. I understand about living life ;)

@braver
Copy link
Collaborator

braver commented Jul 17, 2024

@dennykorsukewitz thanks! Hope that works out.

@braver braver removed the stale The pull request needs to be updated but has not been within the recent past (2 weeks) label Jul 17, 2024
@DavidGerva
Copy link
Contributor

DavidGerva commented Jul 17, 2024

for what it is worth my opinion here :) , LGTM!

@dennykorsukewitz
Copy link
Contributor Author

@dennykorsukewitz thanks! Hope that works out.

He has already replied:

Hi Denny!
Sure, go ahead.

And thank you for your work :)
David

@braver
Copy link
Collaborator

braver commented Jul 18, 2024

Alright, then replacing the existing package is the way forward. Please make the required changes here, and tag release with a higher number than the last release of the old package. It’s always a good idea to link back to the old repo from your repo, and explain the takeover. If you intend to also make changes that are visible to the end user, like changing the command palette entry labels, you might want to consider creating a message for your new version to explain that to any existing users.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: SUCCESS

Packages modified:
  - Add Folder To Project

@dennykorsukewitz
Copy link
Contributor Author

dennykorsukewitz commented Aug 6, 2024

Todo:

@dennykorsukewitz dennykorsukewitz changed the title Added 'Add Folder To Project 2'. Rewrite of 'Add Folder To Project'. Aug 6, 2024
@braver braver added the mergeable The PR is in a mergeable state but awaiting some final comments/acknowledgement from either side label Aug 7, 2024
@dennykorsukewitz
Copy link
Contributor Author

Hi @braver,

Everything is ready, as far as I can see.

#8904 (comment)

🚀

@braver
Copy link
Collaborator

braver commented Aug 8, 2024

🎉

@braver braver merged commit 006c3b1 into wbond:master Aug 8, 2024
2 checks passed
@dennykorsukewitz
Copy link
Contributor Author

Hi @braver

do you have any idea why the package is not displayed on https://packagecontrol.io?

Best Denny

@braver
Copy link
Collaborator

braver commented Aug 8, 2024

@dennykorsukewitz
Copy link
Contributor Author

Ahhh than I have to create a new PullRequest.

Add Folder To Project is not assigned to existing user:
https://packagecontrol.io/browse/authors/Denny%20Korsuk%C3%A9witz

My bad, i will fix that

Best Denny

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback provided mergeable The PR is in a mergeable state but awaiting some final comments/acknowledgement from either side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants