Skip to content

Commit

Permalink
ansi escapes in custom source markers (#12435)
Browse files Browse the repository at this point in the history
* make .rs.scalar() after extrating `html`ness

* AceAnnotation.html()

* rework LintItem.asAceAnnotations() so that they either have html or text

* LintItem may have html or text, not raw

* handle AceAnnotation.html

* don't treat `class = "html"` in custom source markers
  • Loading branch information
romainfrancois committed Jan 6, 2023
1 parent 23c27fe commit c78e9d4
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 32 deletions.
7 changes: 1 addition & 6 deletions src/cpp/r/R/Api.R
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,6 @@

# normalize paths
markers$file <- .rs.normalizePath(markers$file, mustWork = TRUE)

# check for html
markers$messageHTML <- inherits(markers$message, "html")

} else if (is.list(markers)) {
markers <- lapply(markers, function(marker) {
markerTypes <- c("error", "warning", "box", "info", "style", "usage")
Expand All @@ -245,8 +241,7 @@
marker$line <- .rs.scalar(as.numeric(marker$line))
marker$column <- .rs.scalar(as.numeric(marker$column))
marker$message <- .rs.scalar(marker$message)
marker$messageHTML <- .rs.scalar(inherits(marker$message, "html"))


marker
})
} else {
Expand Down
3 changes: 1 addition & 2 deletions src/cpp/session/modules/SessionDiagnostics.R
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@
file = file,
line = x$start.row,
column = x$start.column,
message = x$message,
messageHTML = FALSE
message = x$message
)
})
})
Expand Down
6 changes: 2 additions & 4 deletions src/cpp/session/modules/SessionMarkers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,15 +391,13 @@ SEXP rs_sourceMarkers(SEXP nameSEXP,
std::string path;
double line, column;
std::string message;
bool messageHTML;
Error error = json::readObject(
markerJson.getObject(),
"type", type,
"file", path,
"line", line,
"column", column,
"message", message,
"messageHTML", messageHTML);
"message", message);
if (error)
{
LOG_ERROR(error);
Expand All @@ -411,7 +409,7 @@ SEXP rs_sourceMarkers(SEXP nameSEXP,
FilePath(path),
safe_convert::numberTo<double, int>(line, 1),
safe_convert::numberTo<double, int>(column, 1),
core::html_utils::HTML(message, messageHTML),
core::html_utils::HTML(message, false),
true,
true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import com.google.gwt.user.client.Event;
import com.google.gwt.user.client.Event.NativePreviewEvent;
import com.google.gwt.user.client.Event.NativePreviewHandler;
import com.google.gwt.user.client.ui.Label;
import com.google.gwt.user.client.ui.HTML;
import com.google.gwt.user.client.ui.PopupPanel;
import com.google.gwt.user.client.ui.PopupPanel.PositionCallback;

Expand Down Expand Up @@ -202,7 +202,11 @@ private void displayMarkerDiagnostics(Marker marker)
marker.getRange().getEnd().getRow() >= row)
{
activeMarker_ = marker;
showPopup(annotation.text(), marker.getRange());
String text = annotation.html();
if (StringUtil.isNullOrEmpty(text))
text = annotation.text();
showPopup(text, marker.getRange());

return;
}
}
Expand All @@ -226,7 +230,7 @@ public void onCursorChanged(CursorChangedEvent event)
}
});
addStyleName(RES.styles().popup());
setWidget(new Label(text));
setWidget(new HTML(text));
}

private final Range range_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public AceAnnotation asAceAnnotation()
return AceAnnotation.create(
anchor_.getRow(),
anchor_.getColumn(),
annotation_.html(),
annotation_.text(),
annotation_.type());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,26 @@ protected AceAnnotation() {}

public static native AceAnnotation create(int row,
int column,
String html,
String text,
String type) /*-{
return {
var aceAnnotation = {
row: row,
column: column,
text: text,
type: type
}
};
if (html)
aceAnnotation.html = html;
else
aceAnnotation.text = text;
return aceAnnotation;
}-*/;

public final native int row() /*-{ return this.row; }-*/;
public final native int column() /*-{ return this.column; }-*/;
public final native String text() /*-{ return this.text; }-*/;
public final native String html() /*-{ return this.html; }-*/;
public final native String type() /*-{ return this.type; }-*/;
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public static final native LintItem create(int startRow,
"end.row": endRow,
"end.column": endColumn,
"text": text,
"raw": text,
"type": type
};
Expand Down Expand Up @@ -74,6 +73,14 @@ public final native void setText(String text) /*-{
this["text"] = text;
}-*/;

public final native String getHtml() /*-{
return this["html"];
}-*/;

public final native void setHtml(String html) /*-{
this["html"] = html;
}-*/;

public final native String getType() /*-{
return this["type"];
}-*/;
Expand All @@ -90,6 +97,7 @@ public final AceAnnotation asAceAnnotation()
return AceAnnotation.create(
getStartRow(),
getStartColumn(),
getHtml(),
getText(),
getType());
}
Expand All @@ -102,17 +110,23 @@ public static final native JsArray<AceAnnotation> asAceAnnotations(
for (var key in items)
{
var type = items[key]["type"];
var item = items[key];
var type = item["type"];
if (type === "style" || type === "note")
type = "info";
aceAnnotations.push({
row: items[key]["start.row"],
column: items[key]["start.column"],
html: items[key]["text"],
text: items[key]["raw"],
var annotation = {
row: item["start.row"],
column: item["start.column"],
type: type
});
};
var html = item["html"]
if (html)
annotation.html = html;
else
annotation.text = item["text"];
aceAnnotations.push(annotation);
}
return aceAnnotations;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,7 @@ public AceAnnotation asAceAnnotation()
return AceAnnotation.create(
anchor_.getRow(),
anchor_.getColumn(),
annotation_.html(),
annotation_.text(),
annotation_.type());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import com.google.gwt.dom.client.DivElement;
import com.google.gwt.dom.client.Document;
import com.google.gwt.user.client.Command;

import org.rstudio.core.client.StringUtil;
import org.rstudio.core.client.VirtualConsole;
import org.rstudio.studio.client.RStudioGinjector;
import org.rstudio.studio.client.common.filetypes.TextFileType;
Expand Down Expand Up @@ -98,14 +100,17 @@ public void showLint(JsArray<LintItem> lint)
{
for (int i = 0; i < lint.length(); i++) {
LintItem lintItem = lint.get(i);
DivElement element = Document.get().createDivElement();
VirtualConsole vc = RStudioGinjector.INSTANCE.getVirtualConsoleFactory().create(element);

vc.setPreserveHTML(true);
vc.submit(lintItem.getText());
String renderedText = element.getInnerHTML();
if (StringUtil.isNullOrEmpty(lintItem.getHtml()))
{
DivElement element = Document.get().createDivElement();
VirtualConsole vc = RStudioGinjector.INSTANCE.getVirtualConsoleFactory().create(element);

lintItem.setText(renderedText);
vc.setPreserveHTML(true);
vc.submit(lintItem.getText());
String renderedText = element.getInnerHTML();
lintItem.setHtml(renderedText);
}
}

target_.getDocDisplay().showLint(lint);
Expand Down

5 comments on commit c78e9d4

@netique
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romainfrancois I think either this or aefa767 broke Tab completion for roxygen tags and snippets, at least on macOS. The issue emerged somewhere between 7315386 and 2088e48 (daily builds 41 to 76). When I hit Tab upon typing fun, for instance, fun gets indented and preceding whitespace and characters are selected on subsequent Tab hits.

@romainfrancois
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I will investigate.

@romainfrancois
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can track it down to the changes in aefa767, i.e. https://github.com/rstudio/rstudio-ace/pull/9

@romainfrancois
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in build RStudio 2023.03.0-daily+259 "Cherry Blossom" Daily (a467194, 2023-01-20) for macOS
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) RStudio/2023.03.0-daily+259 Chrome/108.0.5359.179 Electron/22.0.3 Safari/537.36

@netique
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great. Thanks!

Please sign in to comment.