Glavin001 / atom-preview

:construction: (NO LONGER MAINTAINED) :construction: - :eyeglasses: Ultimate previewer of source code in Atom.
https://atom.io/packages/preview
MIT License
51 stars 23 forks source link

Preserve line-breaks in compiler error messages #100

Open darrencruse opened 9 years ago

darrencruse commented 9 years ago

Searched briefly but didn't see this one - apologies if I overlooked it.

I was playing last night and pretty easily added a renderer for LispyScript (which I've been enhancing recently), but one thing I couldn't resolve was that I'd recently made the LispyScript error messages do the thing where it displays the source line of the error, then displays a second line that puts a "^" beneath the column position of the error (imitating how node displays errors).

This error format displays poorly in atom preview, and I notice CoffeeScript's error messages are hard to read for the same reason (it uses that same "^" formatting).

It reads funky because:

a.  the error message is center aligned not left aligned
b.  multi-line error messages are run together i.e. line-breaks/carriage returns are not displayed (I assume because the error is treated as a string and made the text of a dom element?).

I addessed "a." pretty easily with a setting in my ~/.atom/styles.less file:

.text-error { text-align: left; font-size: 22px }

But b. seems a bigger question that will need a code change I'm guessing.

Would people be open to an enhancement to address this? I haven't looked too closely but I'm guessing it might be an easy change I could give it a shot and do a pull request if people are interested.

e.g. maybe a bit of code could be added that snoops the error message for \n and if it finds them replaces them with <br/>, sets that as innerHtml, and changes the css class to use left alignment instead of center?

Or do people think this would mess too much with the formatting of the existing languages?

(I admit it's really just LispyScript and CoffeeScript I've played with in atom preview so far).

Thanks,

Darren

Glavin001 commented 9 years ago

I'd definitely be open to a Pull Request that makes viewing errors much cleaner. Thanks!

darrencruse commented 9 years ago

Here's what I've got at the moment thought I'd post a screenshot first in case you'd like to comment.

I guess the big question is to my eyes this looks better with 100% width and height so it no longer shows the prior preview behind it at all - but let me know if you disagree.

screen shot 2015-06-13 at 8 08 40 am

I still need to run through all the supported langs and make sure it looks good for all of them. For those that don't show the stack trace too maybe it's going to need more left/top margin (so it's not too tiny scrunched up at the top).

Otherwise regarding the coding I'm pretty dang new to coffeescript and even more so to spark-pen - I'm not sure there was a shorter/cleaner way but right I now I found I couldn't make it work unless I used "@raw"

So the main change is just a little css (for the left align and the font sizes) and to this spot in preview-view.coffee:

  showError: (result) ->
    # console.log('showError', result)
    failureMessage = if result and result.message
      '<div class="text-error">' + result.message.replace(/\n/g, '<br/>') + '</div>'
    else
      ""
    stackDump = if result and result.stack
      '<div class="text-warning">' + result.stack.replace(/\n/g, '<br/>') + '</div>'
    else
      ""
    @showMessage()
    @messageView.message.html $$$ ->
      @div
        class: 'preview-spinner'
        style: 'text-align: center'
        =>
          @span
            class: 'loading loading-spinner-large inline-block'
          @div
            class: 'text-highlight',
            'Previewing Failed\u2026'
          @raw failureMessage if failureMessage?
          @raw stackDump if stackDump?

I guess I won't deliver the font size changes in the css btw? Or should I? Just in general the default font sizes were too small on my machine so I'd actually cranked a couple up in ~/.atom/styles.less. But they're supposed to come from the themes right. So I was unsure if I should deliver the larger font sizes that look good on my machine or not.

What's showing in the screen shot above I've likewise just hacked in ~/.atom/styles.less I've yet to constrant them to only affect the preview:

.text-highlight { text-align: left; font-size: 20px } .text-error { text-align: left; font-size: 20px } .text-warning { text-align: left; font-size: 16px } .overlay { left: 0px; margin-left: 0px; width: 100%; height: 100%; }

Glavin001 commented 9 years ago

they're supposed to come from the themes right.

I think that they should be applied from the user's chosen theme. So do not include those kind of changes.

Feel free to start a Pull Request now and we can continue discussions there. Looking great so far! Awesome work. Thanks.