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

WIP : First draft for #31 #41

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

Conversation

kunal-mohta
Copy link
Contributor

This is a first draft for #31

I have used the syntax as topic:site because ~ is an invalid character. If you still want it to be used, we can change it to a valid character.

Currently I have not randomized the order, instead I have predefined the order in the array randomOrder. I will randomize it when other things are finalized.

@fdb
Copy link
Member

fdb commented Feb 28, 2018

Thanks for the change, but this is not exactly what I meant.

There are no fixed names called ":site" or ":one", these are just example variable names, like "foo" or "hamburger". Importantly, you can make more of them. Imagine you're making a news site generator, you could have one single unique topic ordering for the main articles and one unique topic ordering for the sidebar. (so topic:~main and topic:~sidebar)

So topic:one and topic:~one should both be supported. I chose the ~ character since it resembles the NOT operator in C/C++.

@kunal-mohta
Copy link
Contributor Author

Oh!!
I was also wondering that why I isn't there a one keyword! 😅
So the user has the liberty to specify these names ( one, site, etc. )
Are they given names so they can be used elsewhere?
What I mean to ask is what is the reason that user has to provide a name, why not simply write topic: and topic:~?

@fdb
Copy link
Member

fdb commented Feb 28, 2018

I agree, sometimes it's difficult to think of a name because there will be only one of those things. That's why we use the "one" or "site" placeholder name. However, I would still require them to give a note, just to avoid overly cryptic special-characters-only syntax. We don't want to end up as Perl :-)

@kunal-mohta
Copy link
Contributor Author

👍
Let me try and implement it this way then.

@kunal-mohta
Copy link
Contributor Author

@fdb I am facing some problem with making ~ a valid character.
Can you help me with it?

@fdb
Copy link
Member

fdb commented Feb 28, 2018

Huh why? Is this a regex thing?

@kunal-mohta
Copy link
Contributor Author

I don't think so.
Initially I get this error -

Invalid character ~ at position 13 in phrase '<h2>{{ topic:~ }}</h2>'.

So I made a new const TILDEN = '~'; along with these.
And added its handling at places that I felt are relevant like this and this
Which led me to the error -

Invalid syntax: expected end of reference at position 14, but encountered ~ instead.

@kunal-mohta
Copy link
Contributor Author

Okay, I think I figured this out. I will have to include the case for ~ here too, which currently only checks for ALPHANUM.

@kunal-mohta
Copy link
Contributor Author

kunal-mohta commented Feb 28, 2018

@fdb Does this look similar to what you actually wanted?

@fdb
Copy link
Member

fdb commented Feb 28, 2018

@kunal-mohta I appreciate the work, but it's not ready yet: the ordering still needs to happen and some of the code is a bit sloppy (ie. double equals instead of triple equals — read why).

I would ask to please only make a PR when you have a finished, working implementation. We're a small research team and we don't have the resources to guide this process every step of the way.

@kunal-mohta
Copy link
Contributor Author

@fdb I understand. Sorry for the inconvenience.

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