potential fix for issue#4134 #4136

the-dg04 · 2024-10-11T15:59:02Z

Potential fix for the problem

dos/bat/cmd Disk switch "D:" destroys highlight

Resolves #4134

Changes

in src/languages/dos.js line:151 replaced end:'goto:eof' to end:'\n'

Checklist

Added markup tests, or they don't apply here because...
Updated the changelog at CHANGES.md

joshgoebel · 2024-10-12T02:27:59Z

src/languages/dos.js

@@ -148,7 +148,7 @@ export default function(hljs) {
      {
        className: 'function',
        begin: LABEL.begin,
-        end: 'goto:eof',
+        end: '\n',


Ok, that was a weird end condition, but now D: is incorrectly detected as a label, is it not? From what I'm reading labels always start with : not end with it... so could we just update our label matcher to something like?

[start of line]:[label_name]

I think that would wrap this up nicely.

That makes sense, ive made it so that expressions like [start of line]:[label name] are matched as labels and expressions like [start of line]::[something] are not matched as they denote comments.

…nto the-dg04

joshgoebel · 2024-10-13T04:01:40Z

src/languages/dos.js

-    begin: '^\\s*[A-Za-z._?][A-Za-z0-9_$#@~.?]*(:|\\s+label)',
+    begin: '^:',


Wouldn't this be something more like:

/^:[A-Za-z._?][A-Za-z0-9_$#@~.?]*/

Or whatever value characters for a label are?

I'll have to take a closer look to see how this is currently working. This grammar pattern is pretty old... I think we could have a much simpler rule here, or perhaps a multi-match if we truly want the : to be different than the name of the label.

the-dg04 · 2024-10-14T16:12:55Z

I've added multiple possible cases for the use of : in a dos-script. Although I'm a bit muddled on what the relevance values should be for the new symbols. Mind helping me with that?

joshgoebel · 2024-10-20T20:21:28Z

We can just forget about relevance, we'll be dropping it from v12 anyways.

It would be nice to add some markup examples here so I can see which cases you are trying to cover explicitly. Once I have some solid test cases I'll probably clean this up a bit further before merging.

joshgoebel · 2024-10-31T20:53:45Z

src/languages/dos.js

+
+  const DISK_CHANGE = {
+    className: 'symbol',
+    begin: '^[A-Za-z]:\\?$',


This should be a literal regex, not a string.

joshgoebel · 2024-10-31T20:53:48Z

src/languages/dos.js

+
+  // for matching comments starting with ::
+  const COMMENT_2 = hljs.COMMENT(
+    /^::.*$/, /$/,


The begin should start and the end should end, so I think you want:

start: /^::/
end : /$/

The end shouldn't be included in the begin.

joshgoebel · 2024-10-31T20:54:49Z

src/languages/dos.js

+
+  const OUTPUT_REDIRECT = {
+    className: 'symbol',
+    begin: '[1-2]?[>]>{1}\s*[^&\s]+',


Please add a markup test or two for this so I can understand what exactly you're trying to match. This looks a bit overly complex to me, but maybe I don't understand the goal.

joshgoebel · 2024-10-31T20:58:11Z

src/languages/dos.js

+        className: 'function',
+        begin: DISK_CHANGE.begin,
+        end: /\n/,
+        contains: [ hljs.inherit(hljs.TITLE_MODE, { begin: '([_a-zA-Z]\\w*\\.)*([_a-zA-Z]\\w*:)?[_a-zA-Z]\\w*' }) ]


Please explain what this is attempting to do. Are you trying to highlight all path names?

joshgoebel · 2024-10-31T20:58:31Z

src/languages/dos.js

+        className: 'function',
+        begin: OUTPUT_REDIRECT.begin,
+        end: /\n/,
+        contains: [ hljs.inherit(hljs.TITLE_MODE, { begin: '([_a-zA-Z]\\w*\\.)*([_a-zA-Z]\\w*:)?[_a-zA-Z]\\w*' }) ]


Same question, is this another attempt to highlight pathname?

joshgoebel · 2024-10-31T21:05:32Z

The DOS grammar is simple enough we could try adding path highlighting, but we'd do it by recognizing the FULL path... ie something like: (pseudo code)

disk = [A-Z]:\\
path_component = [A-Za-z.....]
[disk]([path component]\\)*[path component]?

So first the disk, followed by any number of \ terminated path segments possibly following be a single non-slash terminated segment... and maybe one sep rule to match non-slash terminated disks...

Does that make sense?

potential fix for issue#4134

1a545fa

joshgoebel reviewed Oct 12, 2024

View reviewed changes

the-dg04 added 4 commits October 12, 2024 11:54

updated dos.js to not misidentify disk change as a label

168c421

updated dos.js to now only match labels and not comments

29b7c24

updated dos.js to now only match labels and not comments

cab894b

Merge branch 'the-dg04' of https://github.com/the-dg04/highlight.js i…

e510344

…nto the-dg04

joshgoebel reviewed Oct 13, 2024

View reviewed changes

added some match cases in dos.js

428e5f3

joshgoebel reviewed Oct 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

potential fix for issue#4134 #4136

potential fix for issue#4134 #4136

the-dg04 commented Oct 11, 2024

joshgoebel Oct 12, 2024

the-dg04 Oct 12, 2024 •

edited

Loading

joshgoebel Oct 13, 2024

joshgoebel Oct 13, 2024

the-dg04 commented Oct 14, 2024

joshgoebel commented Oct 20, 2024

joshgoebel Oct 31, 2024

joshgoebel Oct 31, 2024

joshgoebel Oct 31, 2024

joshgoebel Oct 31, 2024

joshgoebel Oct 31, 2024

joshgoebel commented Oct 31, 2024

		begin: '^\\s[A-Za-z._?][A-Za-z0-9_$#@~.?](:\|\\s+label)',
		begin: '^:',

potential fix for issue#4134 #4136

Are you sure you want to change the base?

potential fix for issue#4134 #4136

Conversation

the-dg04 commented Oct 11, 2024

Potential fix for the problem

Changes

Checklist

Choose a reason for hiding this comment

the-dg04 Oct 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the-dg04 commented Oct 14, 2024

joshgoebel commented Oct 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshgoebel commented Oct 31, 2024

the-dg04 Oct 12, 2024 •

edited

Loading