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

Unify common code for different output styles #734

Closed
mgreter opened this issue Dec 16, 2014 · 2 comments
Closed

Unify common code for different output styles #734

mgreter opened this issue Dec 16, 2014 · 2 comments

Comments

@mgreter
Copy link
Contributor

mgreter commented Dec 16, 2014

Currently we only support nested and compressed output. But these currently do not share a common parent class. IMO we should create such a base class for all output styles, which will implement all common parts.

#717: Main reasons (beside DRY):

  • Abstract the output buffer and provide a simple API
  • Source-maps should be created automatically
  • The UTF-8 charset should be added automatically
  • Makes it easier to implement other output styles

I already made some experiments when implementing the charset feature.
It should be usable as a starting point, if someone else wants to start with it:

#file: output.hpp
#define SASS_OUTPUT

#ifndef SASS_OPERATION
#include "operation.hpp"
#endif

#ifndef SASS_CONTEXT
#include "context.hpp"
#endif

#include <string>

namespace Sass {
  using namespace std;
  struct Context;

  template<typename T>
  class Output : public Operation_CRTP<void, T> {
    // import all the class-specific methods and override as desired
    using Operation_CRTP<void, T>::operator();

  protected:
    Context* ctx;
    string buffer;
    string rendered_imports;
    bool seen_utf8;

    virtual void fallback_impl(AST_Node* n) = 0;

  public:

    Output(Context* ctx = 0)
    : ctx(ctx),
      buffer(""),
      rendered_imports(""),
      seen_utf8(false)
    { }
    virtual ~Output() { };

    string get_buffer()
    {
      return rendered_imports + buffer;
    }

    // append some text or token to the buffer
    // it is the main function for source-mapping
    void append_buffer(const string& data)
    {
      // search for unicode char
      for(const char& chr : data) {
        // abort clause
        if (seen_utf8) break;
        // skip all normal ascii chars
        if (chr >= 0 && chr < 128) continue;
        // prepend charset to buffer
        buffer = "@charset \"UTF-8\";\n" + buffer;
        // singleton
        seen_utf8 = true;
      }
      // add to buffer
      buffer += data;
      // account for data in source-maps
      ctx->source_map.update_column(data);
    }

  };

}

I was not able to implement this outside the header, which is probably because my c++ template foo is not very good. Maybe someone else knows if this is the only/right way?

@am11
Copy link
Contributor

am11 commented Dec 21, 2014

👍 abstract classes FTW!

Speaking of refactoring the code, would it make sense to also restructure the repo: separate .cs and .cpps from .hs and .hpps like so:

root
   \
    libsass
        \
        sources
            \ 
             {  all source files go here, could have subdirectories }
        \
        headers 
            \ 
             {  all header files go here, could have subdirectories matching those under sources }

This way the README would be visible without scrolling down the repo's main page. 😄

@mgreter mgreter added this to the 3.3 milestone Dec 22, 2014
@mgreter mgreter modified the milestones: 3.2, 3.3 Jan 5, 2015
@mgreter
Copy link
Contributor Author

mgreter commented Jan 6, 2015

This issue should (hopefully) be addressed/fixed by #792.
I have created a common Output class from which nested/compressed inherit!

@mgreter mgreter closed this as completed Jan 6, 2015
@mgreter mgreter self-assigned this Mar 10, 2015
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

2 participants