Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Improve entity scopes and related spec #98

Merged
merged 2 commits into from
Aug 15, 2016

Conversation

DavidePastore
Copy link
Contributor

'name': 'punctuation.definition.entity.begin.html'
'2':
'name': 'entity.name.entity.other.html'
'end': '(;)'
Copy link
Contributor

Choose a reason for hiding this comment

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

Parentheses aren't required around this: you can just match the 0th capture instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @50Wliu and thanks for the suggestions. How do you think it should be? I don't know if I understand what you say.

Choose a reason for hiding this comment

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

'end': ';'
'endCaptures':
  '0':

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @MaximSokolov. Fixed in the last commit.

@winstliu
Copy link
Contributor

What does this fix/add? Is there a reason we can't keep the existing match?

@DavidePastore
Copy link
Contributor Author

@50Wliu The existing match doesn't give us the ability to understand if the user is inserting a new entity or the position of the cursor is on the last character (;).
This PR try to add captures that will be used in the autocomplete-html package (atom/autocomplete-html#19) so it can trigger the autocomplete of the HTML entities.

@revelt
Copy link

revelt commented Aug 12, 2016

I'm investigating what's the latest on this PR, as I want to contribute few attributes to be suggested (they're not present in your pull btw) and these commits might overlap with what I want to add.

Half year has passed since latest action!

@winstliu
Copy link
Contributor

It looks like I dropped the ball on this PR. At this point it needs to be rebased. I still am not convinced that we need to switch to a begin/end model as a capture can simply be added for the semicolon.

@DavidePastore
Copy link
Contributor Author

@revelt If you want you can send me a PR to add more entities.
@50Wliu I'm all ears. What do you think we can do to proceed with this PR?

@winstliu
Copy link
Contributor

winstliu commented Aug 12, 2016

Ok, here's what I'm proposing:

'captures':
  '1':
    'name': 'punctuation.definition.entity.html'
  '2':
    'name': 'entity.name.entity.other.html'
  '3':
    'name': 'punctuation.terminator.statement.html' # or something else, iunno
'match': '(&)([a-zA-Z0-9]+|#[0-9]+|#x[0-9a-fA-F]+)(;)'
'name': 'constant.character.entity.html'

Does that work out for what autocomplete-html needs?

@DavidePastore
Copy link
Contributor Author

Thanks for your proposal @50Wliu . The issue with your solution is that your match doesn't find out if you're in the middle position of the entity (i.e. "&a"). I think that (;)' should be optional. What do you think?

@winstliu
Copy link
Contributor

Ok, I see what you're saying now. Your solution looks like the way to go then.

@DavidePastore
Copy link
Contributor Author

@50Wliu What is the next step? Should I rebase?

@winstliu
Copy link
Contributor

Yes please.

@DavidePastore
Copy link
Contributor Author

@50Wliu Rebase done. 🍨

@winstliu winstliu merged commit c55d62d into atom:master Aug 15, 2016
@winstliu
Copy link
Contributor

Thanks, and sorry for the delay!

@DavidePastore
Copy link
Contributor Author

Thanks for merging 😸

@DavidePastore DavidePastore deleted the add-entity-scope branch August 15, 2016 14:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants