javamelody icon indicating copy to clipboard operation
javamelody copied to clipboard

Long lines are not wrapped properly

Open rPraml opened this issue 2 years ago • 3 comments

We are using Ebean & JavaMelody.

The problem is, that ebean generates queries like you see here which do not properly wrap queries which contains a lot of parameters and no spaces like "(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)" It has the effect, that the detailssql table becomes "ultrawide"

As workaround, we can use

document.getElementById("detailssql").innerHTML = 
    document.getElementById("detailssql").innerHTML.replaceAll(/\,([^\s])/g,",​$1")

in the browser console or changing the CSS property .wrappedText { word-wrap: break-word; } to anywhere; (It seems, that (?,?,?....) is not a word, so it will not break.)

I also found out, that adding a invisible space after each comma would make the wrappedText fix introduced in this commit probably obsolete:

@evernat What do you think, would be a good solution?

  • change word-wrap to anywhere (simple, but wraps between words)
  • remove the 'wrappedText' and add invisible spaces instead after certain characters (like ,) and rely, that there are enough commas in SQLs (which normally are).

A good place could be I18N.htmlEncode My suggestion would be to change the method like that if decision 2 is taken

static final Pattern ADD_WHITESPACE_AFTER_COMMA = Pattern.compile("\\,([^\\s])");
...
		if (encodeSpace) {
			result = result.replace(" ", " ");
		} else {
			result = ADD_WHITESPACE_AFTER_COMMA .matcher(result).replaceAll(",​$1")
                }

rPraml avatar Mar 16 '22 10:03 rPraml

ebean sql is not wrapping friendly. I would suggest that space is added in ebean sql. That would still be correct and more readable sql in general even without javamelody and it would be wrappable sql.

Otherwise breaking words in half like in your first solution is not good. It would be much less readable for many sql requests. And I do not think that changing htmlEncode like in your second solution is right, because it would concern a lot of text in javamelody reports. It would be better to change only text printed for requests details.

evernat avatar Mar 27 '22 16:03 evernat

Hello @evernat

ebean sql is not wrapping friendly. I would suggest that space is added in ebean sql.

I partially agree with that. Yes, the generated SQL is not wrapping friendly for JavaMelody, but this may happen also with other frameworks as well. So fixing a UX bug in the framework would be the wrong way as @rbygrave already mentioned.

Otherwise breaking words in half like in your first solution is not good, It would be much less readable for many sql requests.

This is what javamelody currently does. I made a small fiddle where you see, that JavaMelody wraps words, so that they are unreadable https://jsfiddle.net/xtu21d3v/1/

I also found a possible solution with "table-layout: fixed", that would solve both problems

rPraml avatar Mar 28 '22 06:03 rPraml

I've created a PR. Unfortunately, the "table-layout: fixed" option does not work as excepted

rPraml avatar Mar 28 '22 07:03 rPraml

fixed by PR after review some time ago

evernat avatar Jan 05 '23 12:01 evernat