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

Entity.Reference udpate #6

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

Conversation

adscheevel
Copy link

@adscheevel adscheevel commented Jan 24, 2022

added characters to regex pattern for entity.reference to fix highlight issue when ! not referenced between parentheses.

this would not highlight correctly, but is fixed with this commit:
['Forecast'] = N: IF(!Year @= '2021', 1, 0);

this would highlight correctly:
['Forecast'] = N:IF( (!Year) @= '2021', 1, 0);

this always worked and is unaffected by this commit:
['Forecast'] = N: DB('some_cube', !Year, !some_measure);

added characters to regex pattern for entity.reference
added \\s* to regex pattern for when user puts a space between function and left parenthesis
added \\s* to regex pattern for when user puts a space between function and left parenthesis
@adscheevel
Copy link
Author

added two more commits to the rules and process syntax files for situations when a user puts a space between the function and the left parenthesis. I personally don't do that, but have come across code formatted like that and it's nice to have the syntax highlighting still work.

"DB" in both examples below will highlight correctly after the update
DB(...);
DB (...);

@SGarno
Copy link

SGarno commented Jan 26, 2022

Thanks for your contributions!

While I recognize some people do different things (i.e. DB ( versus DB( or ItemSkip(); versus ItemSkip;) the goal here is to operate more along the lines of encouraging code consistency as we are seeing some of the newer languages, like Go, Python, etc.

Interesting side story: I spoke with the lead architects at IBM a number of years back, and they regretted having functions like Now, ItemSkip, ItemReject etc as valid functions without parentheses. At one point they discussed fixing this, but I guess that never happened because too many people use these functions without them.

Convince me otherwise, but I am not sure about adding this space in functions.

Now if we can only get people to indent their code properly! 🤔

@adscheevel
Copy link
Author

Since I haven't said it already, I've so far been a big fan of the tm1helper extension for VS Code and appreciate all your efforts. I'm a recent convert from Sublime and was damn sad to give that up and the syntax package put together by Hermie64. I was very pleased to see this tm1helper extension and it's helped me grow to enjoy VS Code and not miss Sublime so much. Thank you!

And now for some dissent,

While I recognize some people do different things (i.e. DB ( versus DB( or ItemSkip(); versus ItemSkip;) the goal here is to operate more along the lines of encouraging code consistency as we are seeing some of the newer languages, like Go, Python, etc.

In my opinion, the syntax file is not the proper place to enforce a format ideal. As a user of the syntax highlighter, I expect it to work in cases where people have formatted their functions as I would and in cases when they haven't. To have the syntax highlight fail because someone didn't format a rule the way you think it should be done seems a steep price to pay when you're trying to debug someone else's work. I think the snippets files are the proper place to showcase your opinion of the ideal formatting, as you've done. I do disagree with the space after the left parenthesis and before the right one: DB( !Cube, !<>, !<>, !measure ); that are in your snippets file but that's your prerogative, I've simply removed the spaces in my local copy.

I hadn't noticed yet that the non-argument functions wouldn't register in the extension if the () weren't added i.e. ItemSkip(); vs ItemSkip;. I disagree with your approach. IBM may regret not forcing the parentheses, but I would rather the syntax highlighter pattern align solely to the IBM documented syntax of the function or both patterns than only to the one with the () after. I won't annoy you by pushing a commit that updates the behavior of these functions to register for both patterns and will leave it up to you to decide. I will however be updating my local copy to register both ways for the same reason as stated above: I'm bound to run into code that has the () after a function like ItemSkip and code that doesn't; I expect the syntax highlighter to work in either case.

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