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

Feature: introduce variable #639

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ishmum123
Copy link

@ishmum123 ishmum123 commented Feb 15, 2021

PR for #414 .

Would appreciate any help or collaboration...

@ishmum123 ishmum123 marked this pull request as draft February 15, 2021 03:44
@rikvdkleij
Copy link
Owner

@ishmum123 thanks for your contribution. But we can only implement this proper when the layout is properly parsed. A start is made in the layout lexer branch but still a lot of work.
I also have to rebase master on the branch which is also not trivial.

@ishmum123
Copy link
Author

@rikvdkleij if you could list out the steps of lexer, I might be able to help. Also I felt the project currently lacks a proper testing pattern. I am willing to help out

@rikvdkleij
Copy link
Owner

@ishmum123 thanks! Except for the rebase there are no clear steps. It’s a matter of getting the parser in a shape that it will parse Haskell code while individual expressions, where clauses, etc are identifiable on whatever level.

@ishmum123
Copy link
Author

I am currently trying to Intellij Elm. Does the parsing over there help somehow?

@rikvdkleij
Copy link
Owner

Yes, the Elm plugin solution helped us for the solution in the layoutlexer branch. To continue I have to rebase that branch on master.

@ishmum123
Copy link
Author

ishmum123 commented Feb 16, 2021

I am trying to rebase the branch locally. Working incrementally at the moment, i.e. - rebasing a few commits at a time until I hit a commit with high number of conflicts. Might take some time...

@rikvdkleij
Copy link
Owner

@ishmum123 don’t know if you’re still trying to rebase but you can ignore the conflicts of files inside the gen folder and HaskellParser.java because those are generated.
If not then I will give it a try again 😄

@ishmum123
Copy link
Author

I was out of town for a while... I would start working on it today, God willing

@ishmum123
Copy link
Author

This is my take on it - #646 . Feel free to completely discard it if it causes more harm than good

@rikvdkleij
Copy link
Owner

@ishmum123 thanks! I will take a look soon

@rikvdkleij
Copy link
Owner

@ishmum123 it looks okay. Had to make some small changes and regenerate the parser code to get it compiled.

The HaskellParsingTest is failing because of different element for newline.

@ishmum123
Copy link
Author

ishmum123 commented Feb 28, 2021

@rikvdkleij can you please push your changes to the branch? I can look into the HaskellParsingTest on top of that?

@rikvdkleij
Copy link
Owner

@ishmum123 I’ve pushed my changes to your branch already. It’s a PR

Thanks but be aware that the new PSI tree still has to be defined by us.

@ishmum123
Copy link
Author

ishmum123 commented Mar 3, 2021

@rikvdkleij let's carry the discussion over to the other PR?

@rikvdkleij
Copy link
Owner

@ishmum123 yes, good idea

@ishmum123
Copy link
Author

@rikvdkleij left a comment there. Would be great if you could address that please

@CodeByAidan
Copy link

2 year later, ouch!

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.

3 participants