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

Add basic support for (site/project) key prefixing #226

Merged
merged 9 commits into from
Jun 29, 2014

Conversation

Martii
Copy link
Member

@Martii Martii commented Jun 29, 2014

  • Some variable standardization to help me and others be able to read the code better.
  • Moved output whitespace to a variable so we can tweak it when/if necessary.
  • Bug fix on extra white space added for imperitaves. e.g. no data values.
  • Define a few missing variables... would be nice if V8 would warn on this
  • Enable toggling of the true on uniques... e.g. if you toggle it to false it will be ignored.

TODO:

  1. Find the code for collaboration and...
  2. Break @collaborator and @author ... but fix it too. ;)
  3. Prefix @collaborator and @author with oujs... thus making @oujs:author and @oujs:collaborator

This can be merged to enable the prefix routine since it hasn't broken anything else yet. uniques (notice the plural) is the new identifier... however it would be nice to be able to remap collaboration in it. EDIT: Done. It is also here for @OpenUserJs/admin to evaluate and perhaps some cross testing. First step in getting #208 resolved eventually. It took a little while to integrate sizzles adaptation but it seems to be working well.


Can be viewed at this dev tested script. I added jerone to it just now so he can experience the collaboration right now too. :)

Martii added 3 commits June 28, 2014 22:13
* Some variable standardization to help me and others be able to read the code better.
* Moved output whitespace to a variable so we can tweak it when/if necessary.
* Bug fix on extra white space added for imperitaves. e.g. no data values.
* Define a few missing variables... would be nice if V8 would warn on this
* Enable toggling of the `true` on `uniques`... e.g. if you toggle it to false it will be ignored.

**TODO**:
1. Find the code for collaboration and...
2. Break `@collaborator` and `@author` ... but fix it too. ;)
3. Prefix `@collaborator` and `@author` with `oujs`... thus making `@oujs:author` and `@oujs:collaborator`
* Needs total verification that I haven't missed something.
* I do have some redundant tests in for `meta.oujs` existence but I'll remove those on demand.
* Hopefully this is everything needed.

Applies to OpenUserJS#226 and OpenUserJS#208
@Martii
Copy link
Member Author

Martii commented Jun 29, 2014

This last commit appears to be buggy... trying to track down why with no luck... wait a moment... dev with current pro master is producing the same issue of 404 when script editing from a collaborator account and @author (@oujs:author is this pr) is not correctly set... n/m on the buggy text... So I think #208 is the one that is goofing testing up.

So basically this is the same point that pro is at but with prefix enabled.

… probably stay in there since it's an `||`

Applies to OpenUserJS#226 via OpenUserJS#208
* Removed text `Only the author may edit.` from `@author`... that doesn't make sense to me... but did put in `Required to enable Collaboration` change.

Applies to OpenUserJS#226 via OpenUserJS#208
@Martii
Copy link
Member Author

Martii commented Jun 29, 2014

Alright... give it a whirl please... any boogs please report... I've done a few hours of testing it now and think it's solid to give us prefixing support. Does not close #208 but not intended to.

for (key in meta[name]) {
data = meta[prefix][key];
if (data instanceof Array) {
meta[prefix][key].forEach(function (value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

meta[prefix][key] in this line could be data

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixered. Thanks again.

Martii added 2 commits June 29, 2014 04:40
…how this crept back in... SF changed their wiki store... correcting links to the new path and removing from specifics

Applies to the esrs.
* Mentioned in OpenUserJS#226 (comment)

Thanks Jerone :)

Applies to OpenUserJS#226 via OpenUserJS#208
Conflicts:
	controllers/script.js
	views/pages/newScriptPage.html

Resolved... favor newer ones
Martii pushed a commit that referenced this pull request Jun 29, 2014
Add basic support for key prefixing

Merging for sizzle... he can test and deploy but we're lagging behind a bit here.
@Martii Martii merged commit 7bee4c4 into OpenUserJS:master Jun 29, 2014
@Martii Martii removed the PR READY label Jun 29, 2014
@Martii Martii deleted the allowPrefixedMetadataKeys branch June 29, 2014 19:58
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Jun 29, 2014
@Martii Martii mentioned this pull request Jun 29, 2014
@Martii
Copy link
Member Author

Martii commented Jul 18, 2014

Watchpoint:

... regarding // @name and // @description localization.


@arantius (like I said before hazardous to define root keys at times) @johan
@OpenUserJs/owners

Additional extending possibilities: (damage is already done with dash with @run-at so that doesn't really matter I think.)

...

Prefixing with language codes... could apply to single, dual and with date revisions too... usually sortable on language codes first:

// ==UserScript==
// @name My Script
// @description This script even does the laundry!
// @de-DE#name Mein Skript
// @de-DE#description Dieses Skript macht sogar die Wäsche!
// @oujs:author greasemonkey
// @oujs:collaborator arantius
// @oujs:collaborator johan
// ==/UserScript==

...

This is easier to extract I think... could apply to single, dual and with date revisions too... is usually sortable based off type of key: (recommend'ish with my 2 cents)

// ==UserScript==
// @name My Script
// @name#de-DE Mein Skript
// @description This script even does the laundry!
// @description#de-DE Dieses Skript macht sogar die Wäsche!
// @oujs:author greasemonkey
// @oujs:collaborator arantius
// @oujs:collaborator johan
// ==/UserScript==

New delimiter should basically be a character that isn't an existing operator in ECMAScript (ECMA-262 a.k.a ES) ... again damage is done with dash since that's negation and subtraction operator but does provide a perk with being compatible with existing language codes. Should also be basic ASCII e.g. not Unicode to avoid BOMs and UTF decoding of keys. Also compatible with existing prefixing with : for uso, oujs and possibly gf (previously declared by that project) if he ever implements... also should be compatible with detecting run-at last value key... Whatever this new delimiter ends up being it could be used on either side of the existing project/site : prefix.

var key = '@name#de-DE';
var re = /^@(.*?)(?:#(.*))?$/;

alert(key.match(re));

or perhaps a little more detailed approaching Harmony with some of OUJS STYLEGUIDE.md standards:

var key = '@description#de-DE';
var re = /^@(.*?)(?:#(.*))?$/;

var keyName = null;
var langCode = null;

var matches = key.match(re);
if (matches) {
  [, keyName, langCode] = matches;
  alert(
    [
      'key is ' + keyName,
      'language is ' + (langCode ? langCode : 'en-US')

    ].join('\n')
  );
}

Possible second choice is // @name[de-DE]... again optional dash, underscore, or nothing (e.g. deDE1996) ... all for (pseudo) lang codes.

See also:

@Martii Martii changed the title Add basic support for key prefixing Add basic support for site/project key prefixing Jul 18, 2014
@Martii Martii changed the title Add basic support for site/project key prefixing Add basic support for (site/project) key prefixing Jul 18, 2014
Martii pushed a commit to Martii/UserScripts that referenced this pull request Jul 27, 2014
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Jul 22, 2015
* Crashes the server when removing or never existing `@name` but localized ones exist
* Not entirely sure why one needs to `encodedURI` and the other `decodeURI` but redirect has issue ... possibly a current *node* issue
* Change text to import since it's not uploading a script technically

Additionally applies to OpenUserJS#200, OpenUserJS#226 edge case, and a bit from OpenUserJS#655 detection
@Martii Martii mentioned this pull request Jul 22, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug You've guessed it... this means a bug is reported. enhancement Something we do have implemented already but needs improvement upon to the best of knowledge.
Development

Successfully merging this pull request may close these issues.

2 participants