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

[FR] Expose ParserState of currently invoked function #1037

Closed
am11 opened this issue Apr 2, 2015 · 12 comments
Closed

[FR] Expose ParserState of currently invoked function #1037

am11 opened this issue Apr 2, 2015 · 12 comments

Comments

@am11
Copy link
Contributor

am11 commented Apr 2, 2015

As per the discussion that transpired at: sass/node-sass#646 (comment)

As an implementer I want to propagate the contextual information, in particular, the file path, line and column numbers to the end-user so that they know from where the function was called.

@am11 am11 changed the title Feature Request: Provide way to obtain contextual information to custom functions Feature Request: Provide a way to obtain contextual information in custom functions Apr 2, 2015
@mgreter
Copy link
Contributor

mgreter commented Apr 2, 2015

Technically this translates to beeing able to access the ParserState of the currently invoked function.
This is pretty much the same as the API for accessing the import stack (just return a new sruct type).

@mgreter mgreter added this to the 3.3 milestone Apr 2, 2015
@mgreter mgreter changed the title Feature Request: Provide a way to obtain contextual information in custom functions [FR] Expose ParserState of currently invoked function Apr 2, 2015
@chriseppstein
Copy link
Contributor

Related: #1227

@HamptonMakes HamptonMakes changed the title [FR] Expose ParserState of currently invoked function [FR] Expose ParserState of currently invoked function [$100] Jun 8, 2015
@mgreter mgreter modified the milestones: 3.3.1, 3.3, 3.4 Jun 13, 2015
@mgreter
Copy link
Contributor

mgreter commented Nov 7, 2015

This is pretty much possible now, beside fetching line and column numbers!

@mgreter mgreter modified the milestones: 3.3.4, 3.4 Jan 17, 2016
@mgreter mgreter self-assigned this Jan 17, 2016
@mgreter mgreter modified the milestones: 3.3.5, 3.3.4 Feb 23, 2016
@mgreter mgreter modified the milestones: 3.3.6, 3.3.5 Apr 6, 2016
@bdkjones
Copy link

Any updates on this? I'm looking to access the filename and line numbers in my custom function that handles @warn and @debug statements.

@xzyfer
Copy link
Contributor

xzyfer commented Apr 21, 2016

We have no plans to further this until we reach Sass compatibility it 3.4. This increases the maintenance surface area and our hands are full atm.

@mgreter
Copy link
Contributor

mgreter commented Apr 21, 2016

Some ground work was already done and included in libsass, but not that we could yet craft a good and future proof API (we need to further refactor how we internally account for these states).

@bdkjones
Copy link

Could it potentially be easier to simply have @warn and @debug handled by Libsass itself, rather than rely on the custom function approach?

99% of people are just going to write functions that log the message, line number and file of these warnings, right? Since there's no real customization to be done, what benefit is there in relying on the custom function approach? Does it maybe just make more sense to have Libsass log the output like it does for any other error?

Sent from my iPhone

On Apr 21, 2016, at 04:46, Marcel Greter notifications@github.com wrote:

Some ground work was already done and included in libsass, but not that we could yet craft a good and future proof API (we need to further refactor how we internally account for these states).


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@mgreter mgreter modified the milestones: 3.3.7, 3.3.6 Apr 23, 2016
@mgreter mgreter modified the milestones: 3.3.8, 3.3.7 Apr 30, 2016
@mgreter
Copy link
Contributor

mgreter commented May 11, 2016

Both warn and debug already take the same path as other custom functions. So no, it would rather be more difficult to handle warn etc. different that other custom functions.

Quoting from corresponding node-sass issue: sass/node-sass#895 (comment)
I don't think that sass_compiler_get_last_import will work reliably for custom functions. I stated before that the main problem is that libsass does not preserve the information needed ATM. The problem is that we read/parse/resolve the input/import files into an AST tree that will contain a function call op. Once we evaluate/execute that function call, the context of the actual import is no longer available (we only have one ast tree). That is what #1037 is all about. As I also wrote above IMO there is currently only one way to actually get that info via fb->pstate().path already. But that would be just a "crutch" for now, as the import stack should be available to custom functions (as other stuff). But nobody yet came around to actually solve/implement the missing pieces for libsass.

The easiest way I currently see is to add a new "import" stub ast node to the ast tree, so we can re-create the actual import stack in the eval phase where custom functions are actually executed. Once that is done, it should be straight forward to implement the rest. Biggest part here is probably to design a forward compatible and future proof C-API.

@mgreter
Copy link
Contributor

mgreter commented Jan 5, 2017

This was implemented with #2251

@mgreter mgreter closed this as completed Jan 5, 2017
@am11
Copy link
Contributor Author

am11 commented Jan 6, 2017

@mgreter, this is a huge feature and we can do much on consumer side IMO. Kudus! 🏆 🎉

For node-sass, it would require a non-CAPI implementation via V8/nan wrappers, for which I can spare sometime if no one else is taking it. :)

For libsass-net, I will try to take inference with perl-libsass implementation as we did for all the other features in FFI way.

@chriseppstein, @darrenkopp, is it a valid option if we have a dontnet bring-up under Sass organization account? If so, on which repo we can further this discussion? This will give the project some boost and comparatively fast updates. Out of interest, I am willing to contribute to keep all projects under organization in sync, with all these kind of awesome auxiliary features Marcel has implementing 😎

@darrenkopp
Copy link

@am11 fine with me.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 9, 2017

@darrenkopp please join us in Slack https://libsass-slack.herokuapp.com/

@HamptonMakes HamptonMakes changed the title [FR] Expose ParserState of currently invoked function [$100] [FR] Expose ParserState of currently invoked function [$100 awarded] Jan 20, 2017
@HamptonMakes HamptonMakes changed the title [FR] Expose ParserState of currently invoked function [$100 awarded] [FR] Expose ParserState of currently invoked function Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants