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 #7

Open
fentie opened this issue Apr 23, 2011 · 10 comments · Fixed by #11
Open

PHP #7

fentie opened this issue Apr 23, 2011 · 10 comments · Fixed by #11
Assignees

Comments

@fentie
Copy link
Contributor

fentie commented Apr 23, 2011

I'm going through and noting some mismatches in the test file(s). I'll start making pull requests for them as I have time, but this way others can help out as well:

  • double quotes
  • single quotes
  • semicolons
  • scope resolution operator (double colon)
  • new keyword
  • echo keyword
  • class method calls
  • class names
  • HTML tag names (should be bold)
  • parentheses in PHP embedded in HTML
  • < and > in HTML
  • HTML attributes
  • array constructs
  • commas
  • 2nd opening parenthesis on lines 18 & 19 (don't know what they'd be called)

I'm not yet any good at writing Textmate selectors, but I'll fill in the color values as I can.

Thanks for the port.

@deplorableword
Copy link
Owner

Hi, these errors exist in both light and dark?

@deplorableword
Copy link
Owner

  • new keyword

The only selector I can find for this meta.function-call.php which will also select things like function_name() which is suppose to be grey. Hopefully there's some other way to do it.

  • array constructs

Is now fixed in light and dark.

@fentie
Copy link
Contributor Author

fentie commented Apr 23, 2011

I just put the cursor over "new" and hit ctrl-shift-P (Mac) and it gives the selectors that match, so I used that to fill in the XML. You're right too, any issues with the non-gray colors are also in the light version too. I just remember reading that the Dark version was incomplete and it's the one I use, so that's where I put my efforts.

@deplorableword
Copy link
Owner

Any chance you could provide a simple Gist with some PHP which shows up the broken bits? The sample included with Solarized doesn't cover everything.

@fentie
Copy link
Contributor Author

fentie commented Apr 26, 2011

Pretty much everything I listed above was shown in the example file. I'd like a more thorough example too, though, honestly. I'm basically opening the screenshot (http://ethanschoonover.com/solarized/img/screen-php-dark.png) and Textmate with the sample file and noting differences, then fixing them. I have odd hobbies, it seems.

@deplorableword
Copy link
Owner

Do you want to pull in my upstream changes into your fork and see what it fixes, it should whittle down that list a little bit.

@Zegnat
Copy link
Contributor

Zegnat commented Apr 26, 2011

I’m talking about the Dark theme here. Keep in mind I don’t know anything about TextMate theming. I’m just working with the test file and comparing it to the canonical screenshot of Solarized. Let me add some commenting to the list that started this issue:


  • Double quotes — Fixed.
  • Single quotes — Fixed.
  • Semicolons

These (;) are orange in the current TextMate theme but grey in Solarized. punctuation.terminator.expression.php needs to be updated.

  • Scope resolution operator (double semicolon)

I think fentie means the double colon used for calling static functions within classes. These are green in Solarized and orange in this theme. This one might be impossible to fix since both :: and -> use the same TextMate selector (keyword.operator.class.php) but use different colours in Solarized, green and orange respectively.

  • new keyword — Fixed.
  • echo keyword

Currently displayed in green while Solarized uses orange. This is another one for the unable to fix list. TextMate uses a single selector (support.function.construct.php) for several specific functions (or so it seems). An example within the test case is the empty function. This means echo and empty() will always use the same colour in TextMate while Solarized uses different colours.

  • Class method calls — Fixed.
  • Class names

These use a darker base colour in this theme than in Solarized. support.class.php needs to be updated.

  • HTML tag names — Fixed.
  • Parentheses in PHP embedded in HTML

Actually parentheses are overall a little off. Solarized has red for all parentheses while this theme has some of them orange. The problem seems to be that TextMate does not have a selector for parentheses and instead uses the basic text colour value.

  • For source.php.embedded.block.html this is set to orange, resulting in orange parentheses, and
  • for source.php.embedded.line.html this is set to a grey base, resulting in grey parentheses.

Both these selectors would need red as their default colour for parentheses to show correctly. Unless I’m missing some special TextMate setting.

  • < and > in HTML — Fixed.
  • HTML attributes

These use a lighter base colour in this theme than in Solarized. Should be the same colour as HTML’s < and >. entity.other.attribute-name.html needs to be updated. (Or not? I’m not 100% sure.)

  • Array constructs — Fixed.
  • Commas

These suffer of the same problem as aforementioned parentheses. They don’t seem to have their own selector but just take on the base colour. This also means they will always be the same colour as parentheses.

Personally I rather see red parentheses and commas than making both of them grey, as Solarized defines them.

  • Parenthesis within arrays (2nd opening parenthesis on lines 18 & 19)

Because parentheses aren’t matched by TextMate these have taken on the default colour set in meta.array.php. If you want these to be coloured correctly the default colour for meta.array.php needs to be updated to red.


@deplorableword: could you refer to the above emphasised “titles” when you commit patches for them? That way it’ll be easier to keep track. Let’s try to keep this list up-to-date and strike of those that have been fixed. Please let me know which ones have already been fixed so they can be struck off.

If anyone spots another problem with the PHP theme please open a separate issue. That’s the best way to keep problems organised and for deplorableword to close them when they’re solved.

@fentie
Copy link
Contributor Author

fentie commented Apr 26, 2011

Agreed, both quotes are fixed, as are arrays, new, and class method calls. Yes, I meant colon instead of semicolon, sorry.

I still see HTML tags as normal font weight instead of bold, but it's also possible the font I'm using doesn't have a bold, I'll have to verify that later.

@ghost ghost assigned deplorableword Apr 26, 2011
@Zegnat
Copy link
Contributor

Zegnat commented Apr 26, 2011

both quotes are fixed, as are arrays, new, and class method calls.

Marked these as fixed.

I’ve also fixed some spelling faults and added a description to HTML attributes.


@fentie:

I still see HTML tags as normal font weight instead of bold, but it's also possible the font I’m using doesn’t have a bold

I had the same problem. Only after switching font did it become clear. Just redownloaded the theme to make sure and they really are bold.

@deplorableword
Copy link
Owner

The bold text I'm not sure if it's a TextMate or Typeface rendering issue or both.

@Zegnat Thanks for this list, very helpful. I will make single commits which fix these and name them accordingly.

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 a pull request may close this issue.

3 participants