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.

Sign up for GitHub

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

Jump to bottom

Restore support for highlighting XML-encoded plists #1026

Merged
merged 1 commit into from
May 29, 2019
Merged

Restore support for highlighting XML-encoded plists #1026

merged 1 commit into from
May 29, 2019

Conversation

Footpad
Copy link
Contributor

@Footpad Footpad commented Oct 31, 2018

The XML lexer can now read *.plists, and a disambiguation has been added for
*.plist files since now both the Plist and XML lexers can handle them.

The reason this change was made is that the current Plist lexer handles (and
mangles) property lists that are XML-based. Property lists can be encoded as
binary, ASCII, or XML, with most files being XML-encoded.

@Footpad
Restore support for highlighting XML-encoded plists
065f945 
The XML lexer can now read *.plists, and a disambiguation has been added for
*.plist files since now both the Plist and XML lexers can handle them.

The reason this change was made is that the current Plist lexer handles (and
mangles) property lists that are XML-based. Property lists can be encoded as
binary, ASCII, or XML, with most files being XML-encoded.
@vidarh vidarh mentioned this pull request Jan 17, 2019
Closed
@pyrmont pyrmont mentioned this pull request May 11, 2019
Merged
@pyrmont
Copy link
Contributor

pyrmont commented May 29, 2019

@Footpad If most Plists are XML-encoded, I wonder if it would be preferable to add delegation to the Plist lexer so that it delegates XML leading to the XML lexer. Having the XML be able to handle them directly isn't a bad thing but it seems like needing to know you should use the XML lexer rather than the Plist lexer violates the principle of least surprise. What do you think?

@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed discussion-open and removed author-action The PR has been reviewed but action by the author is needed discussion-open labels May 29, 2019
@Footpad
Copy link
Contributor Author

Footpad commented May 29, 2019

I’m not sure what is meant by delegation, is there an example or documentation that illustrates it? The approach I took here was based on the other examples for similar file types that are lexed as XML. It might be cleaner to do something else, but I would need guidance on how to do so.

@pyrmont
Copy link
Contributor

pyrmont commented May 29, 2019

@Footpad Delegation is a mechanism by which one lexer can 'call' another lexer to perform some of the work. The method is defined in the excerpt below.

rouge/lib/rouge/regex_lexer.rb

Lines 355 to 376 in 273de5e

# Delegate the lex to another lexer. The #lex method will be called
# with `:continue` set to true, so that #reset! will not be called.
# In this way, a single lexer can be repeatedly delegated to while
# maintaining its own internal state stack.
#
# @param [#lex] lexer
# The lexer or lexer class to delegate to
# @param [String] text
# The text to delegate. This defaults to the last matched string.
def delegate(lexer, text=nil)
puts " delegating to #{lexer.inspect}" if @debug
text ||= @current_stream[0]
lexer.lex(text, :continue => true) do |tok, val|
puts " delegated token: #{tok.inspect}, #{val.inspect}" if @debug
yield_token(tok, val)
end
end
def recurse(text=nil)
delegate(self.class, text)
end

You can see some examples in the project.

That said, thinking more about it, it's probably not the right approach given that you'd be effectively delegating the entirety of the source. Are Plists ever a mixture of XML and non-XML? Or is it all one or the other?

@dhmoore
Copy link

dhmoore commented May 29, 2019

Plists are either all XML or all binary. XML is the only format that would need lexing. Apple's documentation describes the binary format as optimized and opaque.

@dblessing
Copy link
Collaborator

Wow, can't believe plists have three different formats. Since the format should be all one type in any case, I think this approach is fine. Maybe it would be worth adding some comments in each lexer to make sure in the future someone doesn't come along and say WTF and remove something.

@pyrmont
Copy link
Contributor

pyrmont commented May 29, 2019

@dhmoore wrote:

Plists are either all XML or all binary. XML is the only format that would need lexing. Apple's documentation describes the binary format as optimized and opaque.

It seems like ASCII-based Plists are a third option (this is the type that Rouge's Plist lexer actually scans).

@pyrmont
Copy link
Contributor

pyrmont commented May 29, 2019

@dblessing

Wow, can't believe plists have three different formats. Since the format should be all one type in any case, I think this approach is fine. Maybe it would be worth adding some comments in each lexer to make sure in the future someone doesn't come along and say WTF and remove something.

Yeah, I agree. I'll make an issue.

@pyrmont pyrmont mentioned this pull request May 29, 2019
Closed
@pyrmont pyrmont merged commit 087fd80 into rouge-ruby:master May 29, 2019
@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Footpad @pyrmont @dhmoore @dblessing