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

Add RTL to courses file #6715

Merged
merged 6 commits into from
Feb 26, 2022
Merged

Add RTL to courses file #6715

merged 6 commits into from
Feb 26, 2022

Conversation

AhmedElTabarani
Copy link
Contributor

What does this PR do?

Add RTL to courses file

For resources

Description

Why is this valuable (or not)?

How do we know it's really free?

For book lists, is it a book? For course lists, is it a course? etc.

Checklist:

  • Read our contributing guidelines
  • Search for duplicates.
  • Include author(s) and platform where appropriate.
  • Put lists in alphabetical order, correct spacing.
  • Add needed indications (PDF, access notes, under construction)

Follow-up

  • Check the status of GitHub Actions and resolve any reported warnings!

@AhmedElTabarani
Copy link
Contributor Author

There is old commits here 🤕
I will try to drop them if these will make a problem

@davorpa
Copy link
Member

davorpa commented Feb 11, 2022

There is old commits here 🤕 I will try to drop them if these will make a problem

This is because we use squash as merge strategy. It doesn't bother but if annoys you....

TIP OF THE DAY 🤓 📚 💻 How to refork an upstream without delete your origin repository.

Execute in your local environment:

git fetch --all
git checkout main
git reset --hard upstream/main
git push origin main --force-with-lease

In forked repositories is very common using branches to pull the contributions and leave master/main branch untouched. It prevents more than one headache when merge conflicts appear 😉. Adopt Git Feature Branch Workflow is a good starting point for beginners. 🚀

Copy link
Member

@davorpa davorpa left a comment

Choose a reason for hiding this comment

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

.NET should be addressed too

There are some known minor issues with pipes (markdown table token), eg. one resource in java section. Only perceptive in GitHub Pages (see #5176 (comment))

Should be escaped \| or adressed in LRM #6714 too

@davorpa davorpa added the waiting for changes PR has been reviewed and changes/suggestions requested label Feb 11, 2022
@davorpa davorpa self-requested a review February 11, 2022 14:36
courses/free-courses-ar.md Outdated Show resolved Hide resolved
@davorpa
Copy link
Member

davorpa commented Feb 11, 2022

Great work again with magic entities 🚀 . It works like a charm. Escapes don't belong to this but let's fix it in any way using this PR

After solving .NET alphabetize plugin raise linter errors: https://github.com/EbookFoundation/free-programming-books/runs/5156256718?check_suite_focus=true#step:7:4

Run fpb-lint ./courses/
/home/runner/work/free-programming-books/free-programming-books/courses/free-courses-ar.md
  5:1-38:2  warning  Alphabetical ordering: swap l.28 and l.21  alphabetize-lists  remark-lint

So I'm suspecting that it doesn't remove non-printable magic entities @vhf , it doesn't?

@davorpa davorpa added the linter error Please, correct build errors found by linter! label Feb 11, 2022
@eshellman
Copy link
Collaborator

They're not "Magic entities", they're unicode characters". Why are you putting &lrm in a heading? This will have bizarre effects on search results.

@AhmedElTabarani
Copy link
Contributor Author

@davorpa

is this allow ?
image

@davorpa
Copy link
Member

davorpa commented Feb 11, 2022

@davorpa

is this allow ? image

even more uggly 😟. HTML should be avoided most of times from markdown files. Maybe using raw glyph from charmap.exe: ---> <---
image

@davorpa
Copy link
Member

davorpa commented Feb 11, 2022

They're not "Magic entities", they're unicode characters". Why are you putting &lrm in a heading? This will have bizarre effects on search results.

@vhf @eshellman raw value also fails 😟. so linter doesn't support at all bidirectional languages, some stripes more are needed before alphabetize (\u200E and \u200F)

https://github.com/vhf/remark-lint-alphabetize-lists/blob/ee5f968040acf941c9c4d61fefb2bb1e3b1e8a7b/lib/alphabetical-list-items.js#L5-L14
image

Fixable by vhf/remark-lint-alphabetize-lists#20

@davorpa
Copy link
Member

davorpa commented Feb 11, 2022

After remove LTM in .NET text, linter pass, but renders bad in GHP environment as mentioned in #6715 (review)

@davorpa davorpa added 🗣️ locale:ar Resources addressing "Arabic / العربية" language 🐛 BUG Confirmed bugs, normally at GitHub Pages waiting for changes PR has been reviewed and changes/suggestions requested and removed waiting for changes PR has been reviewed and changes/suggestions requested labels Feb 12, 2022
Copy link
Collaborator

@eshellman eshellman left a comment

Choose a reason for hiding this comment

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

Could I get an explanation of the strategy here?

* [CS50 in Arabic&rlm;](https://www.youtube.com/playlist?list=PLL2zWZTDFZzibJ49gBM2owqCzda8meSNj) - KMR Script
* [CS50 In Arabic&rlm;](https://www.youtube.com/playlist?list=PLnrlZUDQofUv7JE4QIahAyztrQU9bnJmd) - Coders Camp
* [Data Structure&rlm;](https://www.youtube.com/playlist?list=PLwCMLs3sjOY4UQq4vXgGPwGLVX1Y5faaS) - Hard Code
* [Data Structure C++&lrm;&rlm;](https://www.youtube.com/playlist?list=PLsGJzJ8SQXTcsXRVviurGei0lf_t_I4D8) - Mega Code
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't these cancel out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see case 2: #6714

if author name and name of course are in English and you try to RTL it, the author name and name of course will swap

@davorpa davorpa added the conflicts Conflict(s) need to be resolved label Feb 14, 2022
@davorpa
Copy link
Member

davorpa commented Feb 14, 2022

Sorry, conflicts was introduced by #6723. Solving...

@davorpa davorpa removed the conflicts Conflict(s) need to be resolved label Feb 14, 2022
@davorpa davorpa added the conflicts Conflict(s) need to be resolved label Feb 25, 2022
@davorpa
Copy link
Member

davorpa commented Feb 25, 2022

Conflicts again due to #6730

@davorpa davorpa removed the conflicts Conflict(s) need to be resolved label Feb 25, 2022
@AhmedElTabarani
Copy link
Contributor Author

@davorpa, you work very hard 😅, I forget the last issue about what it was? and how you solved it, I want to learn that.

@davorpa
Copy link
Member

davorpa commented Feb 25, 2022

@davorpa, you work very hard sweat_smile, I forget the last issue about what it was? and how you solved it, I want to learn that.

24/7 more or less inside a bunch of FOOS projects 😆.

It was a renaming of all <a name=" with <a id=" to be more HTML5 compliant. Some headings was affected in this PR due to put your LTM marks. E.g C# and C++.

So an upstream main merge was needed taking care to preserve both sides. It was easy because is a guided process (see GitHub docs), perhaps in other situations like deleted or moved files it's not possible using web based graphical interface. Then this other guide is very usefull to address it using command line.

Said that....

TIP OF THE DAY 🤓 📚 💻

How to refork an upstream without delete your origin repository.

Execute in your local environment:

git fetch --all
git checkout main
git branch backup
git reset --hard upstream/main
git push origin main --force

In forked repositories is very common using branches to pull the contributions and leave master/main untouched. It prevents more than one headache when merge conflicts appear 😉. Adopt Git Feature Branch Workflow is a good starting point for beginners. 🚀

@AhmedElTabarani
Copy link
Contributor Author

In forked repositories is very common using branches to pull the contributions and leave master/main untouched. It prevents more than one headache when merge conflicts appear 😉. Adopt Git Feature Branch Workflow is a good starting point for beginners. 🚀

I will do that in the next time 😅❤

@davorpa davorpa self-assigned this Feb 25, 2022
@AhmedElTabarani
Copy link
Contributor Author

Execute in your local environment:

git fetch --all
git checkout main
git reset --hard upstream/main
git push origin main --force

The PR closed, and the commits deleted, I think I messed up every thing, or you merge it in another PR already ?

@davorpa
Copy link
Member

davorpa commented Feb 25, 2022

Execute in your local environment:

git fetch --all
git checkout main
git reset --hard upstream/main
git push origin main --force

The PR closed, and the commits deleted, I think I messed up every thing, or you merge it in another PR already ?

Refork is a destructive operation, moreover if a PR is in the middle 😟 Sorry if missunderstand.

Recovering from refs is advanced process and could be a bit wreid. So is easier make a branch apart, now you have synced with upstream, named eg: arabic-courses-rtl-support recovering content from ref 69e3d0f available at https://github.com/EbookFoundation/free-programming-books/blob/69e3d0fca09ed31cddc9ebd242440c6a746bf247/courses/free-courses-ar.md?plain=1.

Or try to go back

git checkout 69e3d0fca09ed31cddc9ebd242440c6a746bf247
git push origin main --force

davorpa added a commit that referenced this pull request Feb 25, 2022
@AhmedElTabarani
Copy link
Contributor Author

I merged your PR, then in this PR I was can to reopen it again but without any commits, so it was useless to open it, so i tried to refork an upstream again, but I back to the beginning, so i recovered your PR again

now I can reopen it, but it is still there are no commits

@AhmedElTabarani
Copy link
Contributor Author

I apologize very much to you
Dealing with branches and entering into an old commitment and bringing it back and merging it confuses me a little, I didn't deal with it so much
I only know the basics, I guess you already know this, so excuse me if I caused you problems

@AhmedElTabarani
Copy link
Contributor Author

I did it ❤ Mr @davorpa Thanks you 😅

@davorpa
Copy link
Member

davorpa commented Feb 26, 2022

I apologize very much to you
Dealing with branches and entering into an old commitment and bringing it back and merging it confuses me a little, I didn't deal with it so much
I only know the basics, I guess you already know this, so excuse me if I caused you problems

You are welcome 🤗, but it doesn't matter. I hope you've learn a bit more. I'd settle with one reward... share your knowledges with others 😄 as I do🚀

I promise no more conflicts 😜

@davorpa davorpa self-requested a review February 26, 2022 02:37
@eshellman
Copy link
Collaborator

was the sorting issue resolved?

@davorpa
Copy link
Member

davorpa commented Feb 26, 2022

was the sorting issue resolved?

More or less but not yet completely until linter could be fixed.

.NET category LTM change was reverted, so at this moment doesn't matter but is displayed wrong due to that.

So if you want merge it... go ahead. I add it to review backlog later

@eshellman
Copy link
Collaborator

Thanks!

@eshellman eshellman merged commit 2590eab into EbookFoundation:main Feb 26, 2022
@davorpa davorpa mentioned this pull request Feb 26, 2022
39 tasks
@AhmedElTabarani
Copy link
Contributor Author

AhmedElTabarani commented Feb 26, 2022

I want to say that this PR was a battle that i didn't expected it.
I happy to that we discovered some problems and solved most of them, and learning from it to avoid it in the future, thanks you very much ❤️

@davorpa
Copy link
Member

davorpa commented Feb 28, 2022

I want to say that this PR was a battle that i didn't expected it. I happy to that we discovered some problems and solved most of them, and learning from it to avoid it in the future, thanks you very much heart

Now you can refork repo and adopt feature branches, if you want. There are no pending PRs 🤣 😉

@AhmedElTabarani
Copy link
Contributor Author

AhmedElTabarani commented Feb 28, 2022

Now you can refork repo and adopt feature branches, if you want. There are no pending PRs 🤣 😉

I will but not now, there are some new courses appears 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 BUG Confirmed bugs, normally at GitHub Pages linter error Please, correct build errors found by linter! 🗣️ locale:ar Resources addressing "Arabic / العربية" language waiting for changes PR has been reviewed and changes/suggestions requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants