Skip to content
This repository has been archived by the owner on Nov 3, 2020. It is now read-only.

Support highlighting CP DSL errors when they are not ruby syntax issues #205

Merged
merged 2 commits into from
Jan 17, 2016

Conversation

orta
Copy link
Member

@orta orta commented Jan 17, 2016

@alloy @segiddins - incase I'm missing some additional context the I don't know about WRT DSL errors from CP.

Fixes #204

@orta orta added this to the Podfile Editing milestone Jan 17, 2016
@orta
Copy link
Member Author

orta commented Jan 17, 2016

screen shot 2016-01-17 at 13 16 03

@orta
Copy link
Member Author

orta commented Jan 17, 2016

I've been running with this all day and testing on different podfiles, not seen any adverse effects.

orta added a commit that referenced this pull request Jan 17, 2016
Support highlighting CP DSL errors when they are not ruby syntax issues
@orta orta merged commit fb8c13f into master Jan 17, 2016
@alloy
Copy link
Member

alloy commented Jan 17, 2016

The way the message and line info is parsed seems like it's more complex to me than should be needed. An informative error should at least have a proper backtrace, no? It should als hold an unprettified message imp.

@alloy
Copy link
Member

alloy commented Jan 17, 2016

s/imp/imo/

@orta
Copy link
Member Author

orta commented Jan 18, 2016

No, in this case the backtrace is entirely within CocoaPods's source files alas - see the one in #204

@alloy alloy deleted the highlight_more_errors branch January 18, 2016 08:44
@alloy
Copy link
Member

alloy commented Jan 18, 2016

I see Podfile:14 in the second frame of that backtrace, which is where I mean to get the info from.

What I really mean is that there should not be a reason to parse this information from a exception’s message. That’s too brittle imo, as the formatting of that message might easily change. So either get the info from its canonical location (file:line from backtrace) or change the way the info is stored in CP (i.e. have the exception hold the unformatted message as well: Unsupported options '{:exclusive=>true}' for target 'Aerodramus_Example'.) @segiddins might know if this is not already possible.

The only reason the parsing is done in the case of a SyntaxError is because it’s a special kind of exception related to Ruby, namely one during parsing and thus the file never did load.

@orta
Copy link
Member Author

orta commented Jan 18, 2016

Yeah, this all makes sense, will see what @segiddins says about getting the structured message, but made myself an issue

@alloy
Copy link
Member

alloy commented Jan 18, 2016

👍

@segiddins
Copy link
Member

Iirc there's a parse line and offset method on DSLError already?

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

Successfully merging this pull request may close these issues.

3 participants