emacscollective / no-littering

Help keeping ~/.config/emacs clean
GNU General Public License v3.0
635 stars 69 forks source link

mastodon: Theme mastodon-client--token-file #216

Closed yantar92 closed 1 year ago

yantar92 commented 1 year ago

This file is used to store secrets in plstore.el format.

Package repo: https://codeberg.org/martianh/mastodon.el

tarsius commented 1 year ago

What's the reasoning behind not dropping the -file suffix in this case as the conventions call for?

Since this is an option, upstream should probably be asked to rename to mastodon-client-token-file and we should wait for them to do so before merging this.

I think in this case I would want to stick to the .plstore suffix, as it is more specific. I should probably also update the conventions to say that in some cases it might make sense to stick to the suffix used by upstream, instead of dropping it completely or changing to .eld. The rest of the filename I would still change though, so mastadon-client-token.plstore. What do you think?

yantar92 commented 1 year ago

Jonas Bernoulli @.***> writes:

What's the reasoning behind not dropping the -file suffix in this case as the conventions call for?

Simple omission.

Since this is an option, upstream should probably be asked to rename to mastodon-client-token-file and we should wait for them to do so before merging this.

They have this all over the place. And an open issue since 5 years ago. https://codeberg.org/martianh/mastodon.el/issues/206 I doubt that waiting is a productive idea.

I think in this case I would want to stick to the .plstore suffix, as it is more specific. I should probably also update the conventions to say that in some cases it might make sense to stick to the suffix used by upstream, instead of dropping it completely or changing to .eld. The rest of the filename I would still change though, so mastadon-client-token.plstore. What do you think?

The aim is to enforce the right major mode, right?

plstore.el does this by setting file-local variables, and does not define major mode magic, so suffix does not matter all that much in this particular case.

The fact that file format here is Elisp sexp data is implementation detail of mastodon.el. plstore generally allows storing data encrypted as an alternative.

So, probably no extension at all?

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

tarsius commented 1 year ago

They have this all over the place. And an open issue since 5 years ago. https://codeberg.org/martianh/mastodon.el/issues/206 I doubt that waiting is a productive idea.

Agreed, though since you just commented there, let's give it a day or two.

I think in this case I would want to stick to the .plstore suffix, as it is more specific. I should probably also update the conventions to say that in some cases it might make sense to stick to the suffix used by upstream, instead of dropping it completely or changing to .eld. The rest of the filename I would still change though, so mastadon-client-token.plstore. What do you think?

The aim is to enforce the right major mode, right?

Yes, but it is also nice to know what's inside the file just by looking at its suffix. So I still think it is worth keeping.

plstore.el does this by setting file-local variables, and does not define major mode magic, so suffix does not matter all that much in this particular case.

The fact that file format here is Elisp sexp data is implementation detail of mastodon.el.

Sure, but I see no harm in using a meaningful suffix anyway, be that .eld or .plstore. The reason I prefer .plstore is that this is not some package specific suffix. Plstore is a library used by many packages, and someone might come along and wonder, "which of the themed packages make use of plstore?" and then it would be nice if something as simple as find -name '*.plstore' could answer that question.

plstore generally allows storing data encrypted as an alternative.

So, probably no extension at all?

Or rather .eld would be wrong and we have another argument why .plstore should be used.

yantar92 commented 1 year ago

Jonas Bernoulli @.***> writes:

... Plstore is a library used by many packages, and someone might come along and wonder, "which of the themed packages make use of plstore?" and then it would be nice if something as simple as find -name '*.plstore' could answer that question.

Note that plstore.el itself uses .plist extension in the usage examples.

tarsius commented 1 year ago

Note that plstore.el itself uses .plist extension in the usage examples.

That of course changes everything. :roll_eyes:

Okay then, let's drop the suffix. :stuck_out_tongue_closed_eyes:

yantar92 commented 1 year ago

That of course changes everything. roll_eyes

I am not sure if I understand. Was it a sarcasm or just a normal statement?

tarsius commented 1 year ago

Just a normal statement.

(You probably read this in an email client that showed :roll_eyes: instead of a cute simile that rolled its eyes, which probably made it seems more negative than intended. The intended meaning was "oh boy, I have been operating under false assumptions".)

The arguments that I presented in favor of .plstore do not apply to .plist. The latter provides no hints about the content. Furthermore it is overly specific, it is an implementation detail that plstore uses a plist not an alist. If it wasn't for the fact that the file might be encrypted, I would argue that they should start suggesting .eld but since that is not the case and since they have been suggesting .plist for some time now, I would just continue to do so, if I were in their shoes, I think.

No-littering however should drop the suffix, since general we ignore the suffix picked by upstream. (Above I was arguing why we should follow its lead in this case. That would have been an exception; there is always room for those. But the fact that they use .plist, not .plstore, IMO changes everything.)

yantar92 commented 1 year ago

Agreed, though since you just commented there, let's give it a day or two.

No waiting at the end, I guess...

tarsius commented 1 year ago

That briefly slipped my mind, but if they do rename the variable we can update easily. (Of course we would change the variable and file names, and as a consequence you would have to move the file manually if that happens, since there is no mechanism for renaming files automatically.)