press icon indicating copy to clipboard operation
press copied to clipboard

Escaping correctly carriage returns

Open mebibou opened this issue 12 years ago • 12 comments

I'm using a modified version of PlayLessPlugin for my needs on a project that is also using Press. Since I needed to be able to get the compiled output of a file as a String, I create a method in PlayLessEngine to do it, and name my class MyPlayLessEngine.

The problem that I'm having now is that after using MyPlayLessEngine on an HTML page, and going on a different Html page that is using the press plugin, then '\n' are inserted inside the pressed Less files, therefore causing the css output to be wrong and not taken into my page.

So I added a package named press in my code, with a PlayLessEngine within to override the default one, and instead of doing return css.replace("\\n", "\n"); on line 129 I do return css.replace("\\n", "").replace("\n", ""); and then I don't have the problem anymore. Has it been happening to anyone? would there be another way to correct this? thanks

mebibou avatar Apr 03 '13 07:04 mebibou

The less lib was just updated recently, maybe that's causing problems?

dirkmc avatar Apr 03 '13 16:04 dirkmc

I'm not sure, what version should I try then?

mebibou avatar Apr 04 '13 07:04 mebibou

I think we should stick with the latest, I'm just saying that maybe something changed in that lib that is causing the problem... so maybe that's a place to start investigating. This \n issue is tricky, I remember struggling with it before eventually adding this hack to the code.

dirkmc avatar Apr 04 '13 15:04 dirkmc

Ok so in this case maybe the replacement for "\n" that you added in your code could be replaced by a configurable key instead of "\n", so that people that have the same problem as I do could correct it by just replacing "\n" by nothing, don't you think?

mebibou avatar Apr 04 '13 15:04 mebibou

I'm not sure if I understand the problem you're having. Are you saying that in the resulting less code you see a backslash character and an n character, or are you saying you see newlines?

dirkmc avatar Apr 04 '13 15:04 dirkmc

Yes, in the compress files we see the backslash character as it is, they do not appear as new lines, so for exemple I have body {\n} in the output when I shouldn't see it.

The thing is that we are using another modified version of the PlayLessEngine somewhere else because we need to be able to access the protected function protected String compile(File lessFile, boolean compress) {...}, but I renamed the file as MyPlayLessEngine so that it wouldn't interfere with yours. So I'm not sure if it's linked, but after a few calls to the plugin on different html pages the outputs contain backslash characters and the css files cannot be correctly parsed anymore

mebibou avatar Apr 04 '13 15:04 mebibou

Ahhh, ok yeah I think that's most likely your problem. One of the libs included by play-less is probably overwriting the one included by play-press. Try removing the dependency on play-less and also remove play-less from the modules directory in your play installation.

dirkmc avatar Apr 04 '13 16:04 dirkmc

Thanks for the replies. Unfortunately after removing the the other module that was using also the play-less plugin, I just had the error again (which I did not have for some time but appeared again). Here's what the bootstrap.less file looks like : `/*!

  • Bootstrap v2.1.1 *
  • Copyright 2012 Twitter, Inc
  • Licensed under the Apache License v2.0
  • http://www.apache.org/licenses/LICENSE-2.0 *
  • Designed and built with all the love in the world @twitter by @mdo and @fat. */ article,\naside,\ndetails,\nfigcaption,\nfigure,\nfooter,\nheader`

and my dependency file looking like require: - play - play -> logisimayml 1.8 - play -> secure - play -> cobertura 2.4 - org.codehaus.jackson -> jackson-mapper-asl 1.9.10 - org.concordion -> concordion 1.4.2 - com.microsoft -> jdbc4 1.0 - org.hibernate -> hibernate-entitymanager 4.1.6.Final - nl.siegmann.epublib -> epublib-core 3.1 - eu.medsea.mimeutil -> mime-util 2.1.3 - org.apache.commons -> commons-lang3 3.1 - org.json -> json 20090211 - play -> press 1.0.36 - play -> messages 1.3

mebibou avatar Apr 05 '13 14:04 mebibou

hmmm.. Are you sure you removed the less module from play's cache in play/modules?

dirkmc avatar Apr 05 '13 14:04 dirkmc

Yes i removed it, nowhere to be found, and it is still happening... so now when it happens I just modified one line in LessCompiler.java and iI reload and it works, but not very good for production environnement!

mebibou avatar Apr 15 '13 09:04 mebibou

You said "when it happens". Does it not happen every time? Could you tell me exactly what version of play you are running, and what versions of each module you have loaded. I'll try to reproduce it and see if I can work out what's causing it.

dirkmc avatar Apr 15 '13 13:04 dirkmc

No it does not happen all the time, and I couldn't find a way to reproduce it, it just does sometimes, and then either I restart my play server or I change the the file that makes it buggy. I'm running play-1.2.5, and the modules are listed as above in my dependency.yml.

Other than that, I'm using this version of the PlayLessEngine to access the public String compile(...) function:

package utils;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.io.IOUtils;
import org.mozilla.javascript.WrappedException;

import play.Logger;
import play.cache.Cache;

import com.asual.lesscss.LessEngine;
import com.asual.lesscss.LessException;

public class MyPlayLessEngine {

  LessEngine lessEngine;
  Boolean devMode;
  Pattern importPattern = Pattern.compile(".*@import\\s*\"(.*?)\".*");

  MyPlayLessEngine(Boolean devMode) {
    lessEngine = new LessEngine();
    this.devMode = devMode;
  }

  /**
   * Get the CSS for this less file either from the cache, or compile it.
   */
  public String get(File lessFile) {
    String cacheKey = "less_" + lessFile.getPath() + lastModifiedRecursive(lessFile);
    String css = cacheGet(cacheKey, String.class);
    if (css == null) {
      css = compile(lessFile);
      cacheSet(cacheKey, css);
    }
    return css;
  }

  // TODO: Maybe prevent infinite looping here, in case of an import loop?
  public long lastModifiedRecursive(File lessFile) {
    long lastModified = lessFile.lastModified();
    for (File imported : getImportsFromCacheOrFile(lessFile)) {
      lastModified = Math.max(lastModified, imported.lastModified());
    }
    return lastModified;
  }

  protected Set<File> getImportsFromCacheOrFile(File lessFile) {
    String cacheKey = "less_imports_" + lessFile.getPath() + lessFile.lastModified();

    Set<File> files = null;
    cacheGet(cacheKey, Set.class);

    if (files == null) {
      try {
        files = getImportsFromFile(lessFile);
        cacheSet(cacheKey, files);
      } catch (IOException e) {
        Logger.error(e, "IOException trying to determine imports in LESS file");
        files = new HashSet<File>();
      }
    }
    return files;
  }

  protected Set<File> getImportsFromFile(File lessFile) throws IOException {
    if (!lessFile.exists()) {
      return Collections.emptySet();
    }

    BufferedReader r = new BufferedReader(new FileReader(lessFile));
    try {
      Set<File> files = new HashSet<File>();
      String line;
      while ((line = r.readLine()) != null) {
        Matcher m = importPattern.matcher(line);
        while (m.find()) {
          File file = new File(lessFile.getParentFile(), m.group(1));
          if (!file.exists())
            file = new File(lessFile.getParentFile(), m.group(1) + ".less");
          files.add(file);
          files.addAll(getImportsFromCacheOrFile(file));
        }
      }
      return files;
    } finally {
      IOUtils.closeQuietly(r);
    }
  }

  public String compile(String input) throws LessException {
    return lessEngine.compile(input);
  }

  protected String compile(File lessFile) {
    try {
      return lessEngine.compile(lessFile);
    } catch (LessException e) {
      return handleException(lessFile, e);
    }
  }

  public String handleException(File lessFile, LessException e) {
    Logger.warn(e, "Less exception");

    String filename = e.getFilename();
    List<String> extractList = e.getExtract();
    String extract = null;
    if (extractList != null) {
      extract = extractList.toString();
    }

    // LessEngine reports the file as null when it's not an @imported file
    if (filename == null) {
      filename = lessFile.getName();
    }

    // Try to detect missing imports (flaky)
    if (extract == null && e.getCause() instanceof WrappedException) {
      WrappedException we = (WrappedException) e.getCause();
      if (we.getCause() instanceof FileNotFoundException) {
        FileNotFoundException fnfe = (FileNotFoundException) we.getCause();
        extract = fnfe.getMessage();
      }
    }

    return formatMessage(filename, e.getLine(), e.getColumn(), extract, e.getType());
  }

  public String formatMessage(String filename, int line, int column, String extract, String errorType) {
    return "body:before {display: block; color: #c00; white-space: pre; font-family: monospace; background: #FDD9E1; border-top: 1px solid pink; border-bottom: 1px solid pink; padding: 10px; content: \"[LESS ERROR] " + String.format("%s:%s: %s (%s)", filename, line, extract, errorType) + "\"; }";
  }

  private static <T> T cacheGet(String key, Class<T> clazz) {
    try {
      return Cache.get(key, clazz);
    } catch (NullPointerException e) {
      Logger.info("LESS module: Cache not initialized yet. Request to regular action required to initialize cache in DEV mode.");
      return null;
    }
  }

  private static void cacheSet(String key, Object value) {
    try {
      Cache.set(key, value);
    } catch (NullPointerException e) {
      Logger.info("LESS module: Cache not initialized yet. Request to regular action required to initialize cache in DEV mode.");
    }
  }
}

mebibou avatar Apr 22 '13 12:04 mebibou