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

Restore Base Encoder package #9001

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Restore Base Encoder package #9001

wants to merge 1 commit into from

Conversation

poke
Copy link

@poke poke commented Nov 11, 2024

This restores the Base Encoder package which has been marked as “missing“ on the website and has been removed from the channel in #8844.

The original repository is gone but the fork by @socsieng is still available and the original author agreed to have the package point to the fork as part of the discussion in #5016.

- Restore Base Encoder package.
- Adjust repository URL to maintained fork.
@deathaxe
Copy link
Contributor

deathaxe commented Nov 22, 2024

https://packagecontrol.io/packages/StringUtilities at least provide base64 decoder and encoder.

I wonder if it would make more sense to add missing base16 and base32 converters to that package instead.

That said code of existing "Base Encoder" looks rather dated.

If we want to re-add it, I'd suggest to also revise the code base.

  1. make use of edit_settings
  2. opt-in to python 3.8 for ST4
  3. replace the threaded set_status() call by window.status_message()
  4. maybe drop default key bindings as it is easier to add them than to get rid off.

FWIW, we can use SublimeText org to maintain and ship a revised version of that package (or StringUtilities).

@deathaxe
Copy link
Contributor

Also found https://packagecontrol.io/packages/StringEncode with base64 encoder/decoder.

@braver
Copy link
Collaborator

braver commented Nov 30, 2024

"missing" just means "deleted" most of the time, PC's website doesn't handle those very well. Since there are more modern implementations of the same idea already in PC, I don't see a particular reason to bring this one back from the dead?

@FichteFoll
Copy link
Collaborator

FichteFoll commented Dec 2, 2024

From the discussion in the other issue/PR #5016, it appears that there are at least 2 people actively communicating that they are using the package and would like to see it reappear.

@poke, @socsieng (sorry, I accidentally pinged @euphwes earlier): any comments on @deathaxe's review above?

@deathaxe
Copy link
Contributor

deathaxe commented Dec 2, 2024

FWIW, I have a rewritten "Base Encoder" with all my review comments applied.

I just wonder, whether we should instead takeover also unmaintained StringUtilities package to SublimeText org and add missing base encoder commands from "Base Encoder" package instead of restoring this one.

@braver
Copy link
Collaborator

braver commented Dec 4, 2024

@deathaxe that sounds like a great plan 👍🏻

@deathaxe
Copy link
Contributor

deathaxe commented Dec 8, 2024

StringUtilities shares most conversion functions with StringEncode. As I found myself dropping various obsolete functions from StringUtilities I got the impression of StringEncode having been gone the same route a decade before.

As StringEncode also implements an "Paste Encoded..." command to solve the "convert in-place" vs. "insert converted string" challenge without relying on separate commands or package settings and author still seems to be quite active, I decided to create some PRs to push it to python 3.8 and add missing base12 and base16 commands.

@deathaxe
Copy link
Contributor

deathaxe commented Dec 9, 2024

StringEncode 2.7.0 with all base encoding/decoding commands was released.

@braver
Copy link
Collaborator

braver commented Dec 12, 2024

So StringUtilities is pretty much dead, but it's there and I guess it's not hurting anyone. How about instead of bringing Base Encoder back, we now add that to the "previous_names" for StringEncode? Not sure what that will do for people who still have Base Encoder installed, ie. if that would be annoying, but at least it cleans up the registry (by now I assume deletions will never be handled properly by the PC site). @deathaxe?

As for this PR, it seems the original package has been superseded in various ways, and it doesn't make a lot of sense to bring it back. Also considering that there are dozens of ways of installing packages, and even have them auto-update, without them being in package control. If there are 2 users of the package, or maybe even more 😉, they already have options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants