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 Prolog syntax highlighting #2848

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

laalsaas
Copy link

@laalsaas laalsaas commented Feb 5, 2024

I found this Prolog Syntax definition and added it to bat. It is under MPL, so I added support for generating Acknowledgement for that license. Feel-free to double-check the legal part.

I had a bit of a hard time creating this PR because much of the tooling in this repo relies on the bat-binary in $PATH instead of the locally built one. This was troubling when using my own acknowledgement code, as well as when generating the syntax test. I ended up hard-coding the path of the binary into the scripts (which I didn't submit for obvious reasons). I'm mentioning this in case anyone runs into the same problem.

Fixes #1409

@sharkdp
Copy link
Owner

sharkdp commented Feb 23, 2024

Thank you. Can you look into getting the CI tests to pass? (add changelog entry, look into license check)

@laalsaas
Copy link
Author

There are 3 Failing tests:

  • Changelog entry, I made one but I think it's missing the PR Number, no problem, can add that.
  • The conflicting file endings: Prolog and Perl both claim the .pl extension, I guess it's more sane to leave this to Perl. The interesting question is where to change that, should I just update the .sublime-syntax file or also the .tmLanguage file from which it was generated.
  • The License Check: I think Ican't resolve that. The Prolog Syntax is under MPL. The MPL explicitly mentions the GPL in its text. Since the Test just greps for GPL, It fails, even though I didn't include any GPL code.

@sharkdp
Copy link
Owner

sharkdp commented Feb 24, 2024

The conflicting file endings: Prolog and Perl both claim the .pl extension, I guess it's more sane to leave this to Perl.

Agreed.

The interesting question is where to change that, should I just update the .sublime-syntax file or also the .tmLanguage file from which it was generated.

Editing .sublime-syntax only is fine, thank you!

The License Check: I think Ican't resolve that. The Prolog Syntax is under MPL. The MPL explicitly mentions the GPL in its text. Since the Test just greps for GPL, It fails, even though I didn't include any GPL code.

@Enselic Any idea how we could resolve this?

@Enselic
Copy link
Collaborator

Enselic commented Feb 24, 2024

Maybe we could try to use cargo deny for license checks. I use it in other projects and it is more sophisticated than our custom solution. Unsure if it supports submodules though.

If that doesn't work, I think we simply have to make our own check more sophisticated.

@laalsaas
Copy link
Author

laalsaas commented Apr 4, 2024

Hey, is there any chance moving this forward? i.e. would it be an option to add this file to the excluded files?

@Enselic
Copy link
Collaborator

Enselic commented Apr 5, 2024

I think adding a general exception to MPL is a better approach, but since I think this syntax would be the first MPL syntax, I don't want to approve that myself. New licenses in bat needs to be approved by sharkdp in my view.

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

Successfully merging this pull request may close these issues.

Prolog
3 participants