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

🐛 PHP syntax highlighting not working properly #162

Closed
gpressutto5 opened this issue Apr 29, 2020 · 12 comments
Closed

🐛 PHP syntax highlighting not working properly #162

gpressutto5 opened this issue Apr 29, 2020 · 12 comments

Comments

@gpressutto5
Copy link

The syntax highlighting for PHP only works when there's an opening tag <?php in the hunk.

image

<?php

namespace App\Models;

use Grimzy\LaravelMysqlSpatial\Eloquent\SpatialTrait;
use Illuminate\Database\Eloquent\Builder;
public function restaurants(): BelongsToMany
{
     // Adding a comment
     return $this->belongsToMany(Restaurant::class);
}
@dandavison
Copy link
Owner

dandavison commented Apr 29, 2020

Hi @gpressutto5, thanks for the report! Delta uses the same language syntax definitions as bat, and bat gets them from upstream repositories (sublime syntax definitions).

I think I'm seeing the behavior you're describing in bat as well as delta, do you agree? If so then this is neither a Delta bug nor a Bat bug.

But beyond that, I'm not entirely sure what's going on. I see sublime syntax definitions have a header key called first_line_match and that is used in the PHP syntax file used by bat.

But doesn't it seem like that should be irrelevant if the file has the .php extension?

image

@gpressutto5
Copy link
Author

gpressutto5 commented Apr 29, 2020

@dandavison It makes sense when highlighting an entire file because the code is only interpreted as PHP after the <?php opening tag, otherwise it's only plain text. But it doesn't make sense when highlighting only parts of the file because after that tag everything is PHP, so it is best to assume there is an opening tag even if you can't see it in the current hunk, like github does.
The code highlighting works currently will rarely work for PHP the way it is currently.

@dasl-
Copy link

dasl- commented May 7, 2020

I am also seeing this bug.

I believe this is related to sublime syntax's support for embedding languages:

Sublime Syntax files support the notion of one syntax definition embedding another. For example, HTML can contain embedded JavaScript. Here's an example of a basic syntax defintion for HTML that does so:

The sublime syntax file appears to assume we start in html scope until it sees a php opening tag: https://github.com/sublimehq/Packages/blob/759d6eed9b4beed87e602a23303a121c3a6c2fb3/PHP/PHP.sublime-syntax#L18

A php file often contains html that embeds php. Example: https://gist.github.com/dasl-/72242668bf2dde3634802d49a23ef7b6

Other times, a php file may contain pure php and no html, as in the example posted here. I am not sure what the best fix for this would be... it seems like a tricky thing to solve.

Perhaps we could assume that php files do not contain any html and just highlight them as php?

@dasl-
Copy link

dasl- commented May 8, 2020

As a workaround for this issue, you can:

  1. install bat,
  2. Place this file at ~/.config/bat/syntaxes/PHP.sublime-syntax
  3. bat cache --build

This will make delta assume that any file with a .php extension starts in the PHP scope, rather than HTML scope.

credit to @mjec for this fix

UPDATE: if it doesn't work for you, it might be because your versions of delta and bat are incompatible. This might for instance happen if you installed one of them from a package manager that uses outdated versions. Try using the latest versions of both delta and bat. See: #895

@dasl-
Copy link

dasl- commented May 9, 2020

Another improvement: the default Monokai Extended theme doesn't do the greatest job of coloring PHP code.

Instead, you can use a custom monokai theme to get better coloring.

Before ("Monokai Extended"):
monokai extended

After ("Monokai (SL)"):
monokai (SL)

Instructions for how to upgrade your monokai:

  1. install bat
  2. Place this file at ~/.config/bat/themes/Monokai (SL).tmTheme
  3. bat cache --build
  4. Configure delta to use the theme: delta --theme 'Monokai (SL)'

@goodevilgenius
Copy link

goodevilgenius commented Jul 21, 2020

I've also seen this issue, even though bat does proper syntax highlighting.

In this example, I've modified two php files: one is pure PHP, and one is mixed PHP and HTML.

image

Now, if I use bat to simply print those two files, I get this:

image

image

In bat, you can see that the PHP sections are appropriately highlighted. However, in delta, the HTML sections are highlighted as HTML, but the PHP sections are unhighlighted.

@dasl-'s workaround fixed delta for me.

@gpressutto5
Copy link
Author

@goodevilgenius That's because in the first example you have the short open tag <=.
In the second example, when you're using bat you're highlighting the entire file, so it can see the open tag <?php in the start of the file.
I couldn't test the workaround yet, but I will later.

@goodevilgenius
Copy link

@gpressutto5 Good observation. I hadn't actually noticed that the code within the <?= was properly highlighted in the mixed file. I had just noticed that the pure PHP file wasn't highlighted at all.

That's a good catch. That would also explain why bat worked just fine, since it had the entire file, rather than just the diff, to work with.

@dandavison
Copy link
Owner

Somewhat related: #117

rtucek added a commit to rtucek/dotfiles that referenced this issue Aug 5, 2020
adam-ja added a commit to adam-ja/dotfiles that referenced this issue Feb 23, 2023
- Install bat directly from github rather than relying on packages in
  apt which may be out of date for older versions of Ubuntu
- Use a fixed version of the PHP syntax file (see dandavison/delta#162 (comment))
@pprkut
Copy link

pprkut commented Sep 11, 2023

@dandavison Would there be a way to enable the workaround purely in delta, without having to install bat?

@dandavison
Copy link
Owner

You don't need bat at run-time for this. But currently using bat cache --build is the only way to build custom versions of the static syntax-highlighting assets that Delta and Bat use. If that code is available in the bat crate then someone could add the asset-building functionality to delta.

@dandavison
Copy link
Owner

Closing as a bug in upstream sublime syntax support (does anyone know if situation has improved since this discussion?). See also #117

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

No branches or pull requests

5 participants