matthiasblaesing commented on code in PR #9240: URL: https://github.com/apache/netbeans/pull/9240#discussion_r2935597836
########## ide/markdown/src/org/netbeans/modules/markdown/resources/layer.xml: ########## @@ -0,0 +1,82 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + +--> +<!DOCTYPE filesystem PUBLIC "-//NetBeans//DTD Filesystem 1.2//EN" "http://www.netbeans.org/dtds/filesystem-1_2.dtd";> +<filesystem> + <folder name="Editors"> + <folder name="text"> + <folder name="x-markdown"> Review Comment: I think it is an error to mix preview and editor settings. I can see the reasoning to do it here, but from my POV we should at least not use the same mimetype. If we ever want to customize rendering of the markdown syntax we are locked. So what about `text/x-markdown+preview`? ########## ide/markdown/src/org/netbeans/modules/markdown/MarkdownDataObject.java: ########## @@ -105,11 +105,11 @@ @GrammarRegistration(mimeType=MarkdownDataObject.MIME_TYPE, grammar="markdown.tmLanguage.json") public class MarkdownDataObject extends MultiDataObject { - public static final String MIME_TYPE = "text/x-markdown-nb"; + public static final String MIME_TYPE = "text/x-markdown"; Review Comment: In #9271 @lkishalmi did something similar. The question raised there, applies here two: If we mess with the mimetype, please check if there is an official one (I understood @lkishalmi as such). If so, why not use it? ########## ide/markdown/src/org/netbeans/modules/markdown/MarkdownViewerElement.java: ########## @@ -134,15 +144,21 @@ public void changedUpdate(DocumentEvent e) { }; private final Task updater = RP.create(MarkdownViewerElement.this::updateView); + private StyledDocument source; + //Font config update listener + private volatile boolean fontChanged = false; Review Comment: Why is this outside `colorProfileChange`? ########## ide/markdown/src/org/netbeans/modules/markdown/utils/Bundle.properties: ########## @@ -0,0 +1,28 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +CSS_DEFAULT=body {padding:40px;}\ Review Comment: This is bikeshedding: the border feels very large to me. With a full HTML renderer I'd use `1ex` or `1em` (not sure if Swing supports that). ########## ide/markdown/src/org/netbeans/modules/markdown/MarkdownViewerElement.java: ########## @@ -111,6 +118,9 @@ public class MarkdownViewerElement implements MultiViewElement { .set(TablesExtension.APPEND_MISSING_COLUMNS, true) .set(TablesExtension.DISCARD_EXTRA_COLUMNS, true) .set(TablesExtension.HEADER_SEPARATOR_COLUMN_MATCH, true) + // Strikethrough change from del to s tag Review Comment: I assume this is because the swing renderer does understand `s`, but not `del`? ########## ide/markdown/src/org/netbeans/modules/markdown/utils/StyleUtils.java: ########## @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.markdown.utils; + +import java.awt.Color; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import javax.swing.text.AttributeSet; +import javax.swing.text.StyleConstants; +import javax.swing.text.html.StyleSheet; +import org.netbeans.api.editor.mimelookup.MimeLookup; +import org.netbeans.api.editor.mimelookup.MimePath; +import org.netbeans.modules.editor.settings.storage.api.EditorSettings; +import org.netbeans.api.editor.settings.FontColorSettings; +import org.netbeans.modules.editor.settings.storage.api.FontColorSettingsFactory; +import org.netbeans.modules.markdown.MarkdownDataObject; +import org.openide.util.NbBundle; + +/** + * + * @author bhaidu + */ +public class StyleUtils { + + private static final Map<String, String> fontConfig2tagMapping = Map.ofEntries( + Map.entry("body", "body"), // NOI18N + Map.entry("code", "code"), // NOI18N + Map.entry("pre", "pre"), // NOI18N + Map.entry("blockquote", "blockquote p"), // NOI18N + Map.entry("heading1", "h1, h1 a"), // NOI18N + Map.entry("heading2", "h2, h2 a"), // NOI18N + Map.entry("heading3", "h3, h3 a"), // NOI18N + Map.entry("heading4", "h4, h4 a"), // NOI18N + Map.entry("heading5", "h5, h5 a"), // NOI18N + Map.entry("heading6", "h6, h6 a") // NOI18N + ); + + public static void addNbSyles(StyleSheet ss) { + + //main css + appendDefaultMarkdownCssRules(ss); + + //coloring & font settings + appendColoringCssRulesFromFontConfigs(ss); + } + + public static void addRule(StyleSheet ss, AttributeSet attributeSet, AttributeSet defaultAttributes) { + String nameAttr = (String) attributeSet.getAttribute(StyleConstants.NameAttribute); + + String htmlTag = fontConfig2tagMapping.get(nameAttr); + + if (htmlTag == null) { + return; + } + + StringBuilder cssRule = new StringBuilder(); + cssRule.append(htmlTag); + cssRule.append(" {"); // NOI18N + cssRule.append(rgbStyling("color", getThemeColor(attributeSet, defaultAttributes, StyleConstants.ColorConstants.Foreground))); // NOI18N + cssRule.append(rgbStyling("background-color", getThemeColor(attributeSet, defaultAttributes, StyleConstants.ColorConstants.Background))); // NOI18N + cssRule.append("}"); // NOI18N Review Comment: The usage of editor settings implies that this would also control the font. This does not seem to be the case. ########## ide/markdown/src/org/netbeans/modules/markdown/resources/Coloring.md: ########## Review Comment: If preview does not work, wouldn't it make sense to only keep lines 1-7? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
