8000 fixed issue #514 by tanmay-sinha · Pull Request #524 · cp-algorithms/cp-algorithms · GitHub
[go: up one dir, main page]
Skip to content

fixed issue #514 #524

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 10, 2020
Merged

fixed issue #514 #524

merged 4 commits into from
Mar 10, 2020

Conversation

tanmay-sinha
Copy link
Contributor
@tanmay-sinha tanmay-sinha commented Mar 10, 2020 •
edited
Loading

changed l; r to l, r in required places (not in any code)(#514 )

jakobkogler
jakobkogler approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member
@jakobkogler jakobkogler left a comment

Choose a reason for hiding this comment

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

Thanks for the work.

@jakobkogler
Copy link
Member

this fixes #514
8000

@jakobkogler jakobkogler merged commit 2657e19 into cp-algorithms:master Mar 10, 2020
@etanot
Copy link
etanot commented Mar 12, 2020

@tanmay-sinha, Thanks for MR, but at many places ; is still being used in place of ,. Mostly where range doesn't start with l, and end with r; and also range notation is still inconsistent that's at some places you've put blank space after , and some don't (see, 48aa8ec#diff-407d49a55f134fb58f26123a78c53202L86). @jakobkogler, can you give me some insight about above proposed changes?

PS: If you don't mind, I can propose a MR which fixes those issues.

@tanmay-sinha
Copy link
Contributor Author
tanmay-sinha commented Mar 12, 2020 •
edited
Loading

@finn0 Yes actually I know that many places still have this issue. It is completely fine if you make a PR . BTW, I am interested in knowing how will you change all the places . Any scripts or else?

@etanot
Copy link
etanot commented Mar 12, 2020 via email

@jakobkogler
Copy link
Member

@finn0 If you have the time, please fix the remaining places with a wrong notation. I haven't actually checked if there are any remaining ones left, but only checked the actual changes from @tanmay-sinha.

Not sure how effective Regex is in such a task, since also half-open intervals like (a,b] and [a,b) , and open intervals like (a,b) should contain a comma.

About the blank space: Latex (or mathjax) will determine automatically how much space is between the comma and the second term. You can quickly test that for yourself by rendering the following code at https://cp-algorithms.com/test.php:

$(a, b)$

$(a,b)$

$(a,    b)$

This will work for all intervals inside mathematical environments. For other intervals (I'm guessing in older articles it might be written with backticks like `[a;b]`), it would be probably best to convert it to a mathematical environment using the dollar symbols. Otherwise I guess I prefer one space after the comma.

ChamanAgrawal pushed a commit to ChamanAgrawal/e-maxx-eng that referenced this pull request Oct 19, 2020
@tanmay-sinha @ChamanAgrawal
fixed issue cp-algorithms#514 (cp-algorithms#524)
a9503e9 
* Fixed typo in stack modification

* changed [l;r] to [l,r] in required places.

* Fixed some more range issues.

* added missing comma
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakobkogler jakobkogler jakobkogler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tanmay-sinha @jakobkogler @etanot
0