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

Grab old UDL files into new location #11

Merged
merged 18 commits into from
Feb 5, 2020
Merged

Grab old UDL files into new location #11

merged 18 commits into from
Feb 5, 2020

Conversation

pryrt
Copy link
Contributor

@pryrt pryrt commented Feb 4, 2020

closes #7

  • populate UDLs from old UDL list (fixing names/ids to match our requirements)
  • update JSON and Markdown lists to match

* DONE: removed the autocomplete's that were in that list (that I've found so far)
* IN PROGRESS: into the M's in commun-udl-list.json
* TODO: other-udl-list
… more effort on my part. Need to decide whether to give up on those, or download and do the local renames like I did on the `commun-udl-list.json`
…import the separate json files into the official list
@pryrt pryrt requested a review from a team February 4, 2020 20:33
@pryrt
Copy link
Contributor Author

pryrt commented Feb 4, 2020

@chcg ,

With the schema checks, are we just disallowing unicode characters completely? Or is there some other way that I don't know of: I tried UTF-8, UTF8-with-BOM, and \uXXXX encoding them, and all failed the check.

(the original authors of the ancient UDLs sometimes had unicode characters in their names, so I was trying to maintain their own name spelling in the author field.)

@pryrt pryrt requested a review from chcg February 4, 2020 21:22
UDLs/readme.txt Outdated Show resolved Hide resolved
udl-list.json Outdated
"version": "Sat, 11 Aug 2012 12:04:24 GMT",
"repository": "",
"description": "Snort",
"author": "http://www.tropismgroup.org/2012/08/02/snort-user-defined-language-udl-in-notepad/ <http://www.tropismgroup.org/2012/08/02/snort-user-defined-language-udl-in-notepad/>"
Copy link
Member

Choose a reason for hiding this comment

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

The content of URL is not readable.
Let's put Anonymous, and remove the URL for the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

URL removed. Since I had called it Snort_by-tropismgroup, I decided to use same identifier as the author.
pryrt@6f8f59e

Copy link
Member

Choose a reason for hiding this comment

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

I don't even think tropismgroup it's accurate.
http://www.tropismgroup.org/ is a lottery website and people need to login for some operation.
So let's put Anonymous until people come to claim it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pryrt@e70fd6c switched to anonymous

udl-list.md Outdated
| [Skyrim Papyrus language](./UDLs/Skyrim-Papyrus_byJohnKostrzewski.xml) | Skyrim Papyrus language | John Kostrzewski |
| [SmallWorld Magik](./UDLs/SmallWorldMagik_byChristianTaton.xml) | SmallWorld Magik | Christian Taton |
| [Smarty](./UDLs/Smarty_byHelge-deVries.xml) | Smarty | Helge de Vries |
| [Snort](./UDLs/Snort_by-tropismgroup.xml) | Snort | http://www.tropismgroup.org/2012/08/02/snort-user-defined-language-udl-in-notepad/ |
Copy link
Member

Choose a reason for hiding this comment

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

The content of URL is not readable.
Let's put Anonymous, and remove the URL for the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

URL removed. Since I had called it Snort_by-tropismgroup, I decided to use same identifier as the author.
pryrt@6f8f59e

Copy link
Member

Choose a reason for hiding this comment

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

Item above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pryrt@e70fd6c switched to anonymous

udl-list.json Outdated
"version": "Sun, 03 Feb 2013 10:32:25 GMT",
"repository": "",
"description": "AVISynth",
"author": "\u041c\u0443\u0441\u0430-\u0410\u0445\u0443\u043d\043e\0432 \u0420\u0443\u0441\u0442\u0430\u043c <mailto:rustam-musa@rambler.ru>"
Copy link
Member

@donho donho Feb 4, 2020

Choose a reason for hiding this comment

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

The JSON validation failed here.
@pryrt Are you sure it's a valid Unicode string?
Without the knowledge of this encoding, I saw the format \uXXXX but these 2 \043e & \0432 don't start with u
@chcg If it's a valid Unicode string, is it possible to modify the build script to validate also Unicode string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@donho, thanks for finding those two. When they were fixed, it moved past the unicode parsing for the validation. I'll work on fixing the other validation errors next.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as of pryrt@7f2f5ba, validates correctly

@donho
Copy link
Member

donho commented Feb 5, 2020

Awesome! Thank you @pryrt for your tremendous effort to make this list available!

One thing I'm worrying about, it's the agreement of author to make their UDLs available in this repository. So we should add a notice for people who prefer to erase the files (even the refs from the list) from this repository. But this notice note could be added after the merge of PR.

@pryrt
Copy link
Contributor Author

pryrt commented Feb 5, 2020

But this notice could be added after the merge of PR.

Should it go in README, CONTRIBUTING, or both?

Normally I would have said CONTRIBUTING, but if we want it to be more visible, it could go in README or both.

@pryrt
Copy link
Contributor Author

pryrt commented Feb 5, 2020

@chcg,

I can easily go in and fix the missing homepage attribute (and probably will). But with the phrasing on the CONTRIBUTING page (emphasis added):

The homepage attribute could be a link to your GitHub repository for the UDL language or your GitHub user page.

Did we want to make that attribute required, or optional?

@donho
Copy link
Member

donho commented Feb 5, 2020

Did we want to make that attribute required, or optional?

IMO it should be optional.

@pryrt pryrt merged commit e3e486a into notepad-plus-plus:master Feb 5, 2020
@chcg
Copy link
Contributor

chcg commented Feb 5, 2020

@pryrt Do you think the schema is ok or is it necessary to change something? In general UTF-8 should be possible for json.

@pryrt
Copy link
Contributor Author

pryrt commented Feb 5, 2020

@pryrt Do you think the schema is ok or is it necessary to change something? In general UTF-8 should be possible for json.

Well, it seemed to be getting hung up on this entry:

{
	"name": "npp-UDL-list",
	"version": "1.0",
	"UDLs": [
		{
			"id-name": "AVISynth",
			"display-name": "AVISynth",
			"version": "Sun, 03 Feb 2013 10:32:25 GMT",
			"repository": "",
			"homepage": "",
			"description": "AVISynth",
			"author": "Муса-Ахунов Рустам <mailto:rustam-musa@rambler.ru>"
		}
	]
}

... where it wouldn't work, whether that JSON was saved as UTF8 or UTF8-with-BOM in Notepad++, it gave that same error. When I used the \uXXXX notation, all of those characters worked and it moved on to validating properties instead of complaining about unicode.

If you think it should handle UTF-8, then there is something that needs to be fixed.

@pryrt pryrt deleted the GrabOldUdl2 branch February 6, 2020 14:10
@chcg
Copy link
Contributor

chcg commented Feb 10, 2020

@pryrt See #15 which corrects the utf8 encoding issue

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

Successfully merging this pull request may close these issues.

Old UDL list from Internet Archive
3 participants