Skip to content

Add support for a single quote in a character literal and a double qu…#57

Open
skaupper wants to merge 1 commit intoPaebbels:devfrom
skaupper:bugfix/vhdl_character_and_string_literals
Open

Add support for a single quote in a character literal and a double qu…#57
skaupper wants to merge 1 commit intoPaebbels:devfrom
skaupper:bugfix/vhdl_character_and_string_literals

Conversation

@skaupper
Copy link
Copy Markdown

@skaupper skaupper commented Jul 3, 2023

As discussed in #56, this PR adds support for single quotes in a character literal (''') and double quotes in a string literal ("""", "foo""", etc.).

As soon as #56 is merged, this PR will get additional test cases (like character'(''')) to avoid conflicts with qualified expressions.

yield previousToken
tokenKind = cls.TokenKind.OtherChars

start = SourceCodePosition(row, column, absolute)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The following if-elif code block is now present about eight times in this function. What are your thoughts on replacing those with calls to a separate function?

The variables buffer, previousToken, tokenKind etc. would have to be passed by reference in order to support yielding tokens out of it. But I do not really know a simple, non-hacky way to pass enumerations by reference (other than wrapping it manually in a temporary object/dict ofc).

Comment thread pyVHDLParser/Token/Parser.py Outdated
tokenKind = cls.TokenKind.OtherChars
else:
continue
continue # TODO: Merge with changes from #56!
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This if-block will be merged with the changes made by #56.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

#56 was merged. What's next for this PR? Anything I can do?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

As soon as I have some spare time I will rebase this branch. Since it touches the same if-branches in the parser as #56 I will have a look if something can be cleaned up.

I would appreciate your thoughts about my other comment, though.

@skaupper skaupper force-pushed the bugfix/vhdl_character_and_string_literals branch from 8cd5ca2 to 63fb610 Compare July 3, 2023 09:25
@Paebbels Paebbels mentioned this pull request Jul 8, 2023
@Paebbels
Copy link
Copy Markdown
Owner

Oh. looks like I got distracted from that one ...

@skaupper skaupper force-pushed the bugfix/vhdl_character_and_string_literals branch from 63fb610 to 479005d Compare August 31, 2023 08:36
@skaupper skaupper marked this pull request as ready for review August 31, 2023 08:40
@skaupper
Copy link
Copy Markdown
Author

As far as I am concerned, this PR could be merged. Do you have any objections?

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.

2 participants