Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matlab class properties #1466

Merged
merged 9 commits into from
Jan 18, 2021
Merged

Matlab class properties #1466

merged 9 commits into from
Jan 18, 2021

Conversation

drmoose
Copy link
Contributor

@drmoose drmoose commented May 29, 2020

Given the following Matlab code,

classdef Name < dynamicprops
    properties
        % i am a comment
        name1
        name2
    end
    methods
        % i am also a comment
        function self = Name()
            % i am a comment inside a constructor
        end
    end
end

pygments' matlab lexer gets very confused indeed. This attempts to improve the situation. Although it does not capture all of the properties syntax, it special-cases properties blocks to prevent the list of names from being misinterpreted as the "command syntax." It also prevents the comments from being mistaken for string literals.

Let me know anything I can do to improve the PR. In particular, if you'd like support for the full properties syntax, I can do that, but I wasn't sure of the best way to re-use token rules from the 'root' in 'defprops' while maintaining readability.

@Anteru Anteru self-assigned this May 29, 2020
@Anteru
Copy link
Collaborator

Anteru commented May 29, 2020

You can separate out rules and include them, which makes it easy to reuse them in various places. See https://github.com/pygments/pygments/blob/master/doc/docs/lexerdevelopment.rst, the example at "You can include the rules of a state in the definition of another. "

@drmoose
Copy link
Contributor Author

drmoose commented Jun 8, 2020

Using that construct to try to get closer to full syntax support, I find there are some inconsistencies with whitespace that I'm not sure how best to address.

Should spaces in the code be Token.Text or Token.Text.Whitespace? And, should contiguous whitespace be a single token ((Token.Text, '\n ')) or one token per character ((Token.Text, '\n'), (Token.Text, ' '), ...)?

@Anteru
Copy link
Collaborator

Anteru commented Aug 22, 2020

Whitespace should be Text.Whitespace, that's what the more modern lexers tend to do anyways. I'd try to merge whitespace if possible but there's no requirement to do so. (I just realized this is also documented in tokens.rst.)

@apjanke
Copy link

apjanke commented Jan 18, 2021

Ping. @drmoose, are you still working on this?

@drmoose
Copy link
Contributor Author

drmoose commented Jan 18, 2021

@apjanke Oh whoops. This thread completely slipped my mind. I've got a bunch of uncommitted changes and I was fighting with the inconsistent treatment of whitespace when I got distracted by other things .... aaaand that was apparently four months ago. Linear time is annoying.

Let's see how close I can get this to commit-worthy today...

@apjanke
Copy link

apjanke commented Jan 18, 2021

That'd be great; thanks! It would be nice to have this to build on for my work on #1684.

@drmoose
Copy link
Contributor Author

drmoose commented Jan 18, 2021

@apjanke Do you have the code snippet you posted in #1684 available in a form other than a screenshot? I'd like to run through my local branch as a test case. I've made no effort to support the arguments syntax but I'm pretty sure I've got property constraints working.

Here's the test string committed in 53ca36c rendered to my console:

image

@apjanke
Copy link

apjanke commented Jan 18, 2021

Oh, sure! Sorry for not posting code as text too, originally. I should know better.

classdef SomeClass < SomeOtherClass

  properties
    x (1,1) double = 42
    y
  end

  methods
    function this = SomeClass()
    end
  end

end

function anExampleFunction(foo, bar)
  arguments
    foo (1,1) double
    bar string = "whatever"
  end

  fprintf('Hello, world!\n')
end

It's from https://github.com/janklab/MatlabProjectTemplate/blob/main/doc-src-jekyll/index.md.

@drmoose
Copy link
Contributor Author

drmoose commented Jan 18, 2021

image

Not bad. As expected, arguments could be better, and it's getting confused about the type annotations

@apjanke
Copy link

apjanke commented Jan 18, 2021

Here's a more thorough version:

classdef SomeClass < SomeOtherClass

  properties
    x (1,1) double = 42
    y
  end

  methods
    function this = SomeClass()
    end
  end

end

function anExampleFunction(foo, bar, baz, qux)
  arguments
    foo
    bar (1,1) double
    baz string = "whatever"
    qux string = "foo" {mustBeMember(qux, ["foo" "bar" "baz"])}
  end

  fprintf('Hello, world!\n')
end

@apjanke
Copy link

apjanke commented Jan 18, 2021

Cool. If you want to leave off here, I can pick up on arguments and type annotations.

@drmoose
Copy link
Contributor Author

drmoose commented Jan 18, 2021

That's probably a good idea as I'm not terribly familiar with either syntax. Fork me if you want, but you now also have write access on my branch. :)

@apjanke
Copy link

apjanke commented Jan 18, 2021

Hopefully you can just get this PR merged soon, and then I can work off of that. I'd rather not do separate PRs or pushes to your branch/PR – that can get messy, and I'm going to be doing a lot of screwing around and testing with this code.

@Anteru Anteru merged commit 423c44a into pygments:master Jan 18, 2021
@Anteru Anteru added this to the 2.8 milestone Jan 18, 2021
@Anteru Anteru added the changelog-update Items which need to get mentioned in the changelog label Jan 18, 2021
@Anteru
Copy link
Collaborator

Anteru commented Jan 18, 2021

@apjanke Please go ahead :)

@Anteru Anteru removed the changelog-update Items which need to get mentioned in the changelog label Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants