search

alan-if / alan-i18n

ALAN Internationalization Project
Other
0 stars 1 forks source link

Corrections and add new test in dev_alan_es #38

Closed Rich15 closed 2 years ago

Rich15 commented 2 years ago

Main changes

Translation and orthography

I've translated all the sentences with the @TRANSLATE mark.

This included several lines in the mensajes.i module that were untranslated or mistranslated. Also changed "salvar" (save) for the better "guardar".

Translated "está/n wearing" to "lleva/n puesto" in examinar.i, but the code should check for gender and number in "puesto". This could be handled when the grammar module is created.

Changed "No puedes llevar" to "No puedes ponerte" so it wouldn't be confused with "You can't carry" (llevar.i)

Translated some more brief sentences and fixed some little orthographic issues (basically accent marks).

New ponibles_test

The test

Added a little test adventure to check for GNA issues and some improvements. It consists of one location and three NPC's.

Issues with the test

It would be great for the Spanish library to have a grammar initialization module, as @tajmone has commented (see #35). I think this would solve the actors' gender problem and deal with other adjectives that need it. By now I haven't mastered ALAN enough to manipulate a entire module, but I'd try to test some little modifications and see how they work.

Tell me any issue or improvement that could be made, specially regarding the script and transcript files (whether they are correctly written or not).

tajmone commented 2 years ago

Need to Rebase!

@Rich15, your PR branch is behind the dev_alan-es branch by four commits (you probably forgot to git pull in dev_alan-es before benching).

All you need to do is:

  1. Checkout dev_alan-es.
  2. git pull from upstream repository (i.e. this one), so you'll update the missing commits.
  3. Checkout dev_alan-es_rich.
  4. Rebase it on dev_alan-es.
  5. Force push the PR branch again (git push -f).

Hopefully, you won't meet any conflicts to solve (i.e. if you haven't edited files which were also changed in those later 4 commits). If there are conflicts, you'll have to manually resolve them via a three way merge, keeping the changes from upstream (i.e. ours/--ours) when you didn't change those line, and choosing your changes (i.e. --yours) when you did change those lines.

Unfortunately, in development branches rebasing is a common operation, because the base dev branches in this repository themselves are often rebased on main, and because by the time you (and anyone else of us) finishes working on a certain feature the base branch might have gone forward in the meantime.

When I'm working on a dev branch, at the beginning of every session I fetch/pull from upstream so if there are changes I just rebase my dev branch straight away, instead of waiting that the work is finished (less conflicts to deal with at once).

So rebasing is normal (and good), so much so that we only rebase pull requests here, we don't merge them nor squash them.

Rich15 commented 2 years ago

Problem with rebase

Thanks for explaining the procedure @tajmone. I pulled the dev_alan-es branch and also my own branch dev_alan-es_rich (I had made some changes from Github).

Now I'm having a problem with the rebase part. It says I can't rebase because I have unstaged changes; I run git status and confirm there are three untracked files. First I try to stash them with git stash but when I check again they're still there. I add the --include-untracked but nothing changes. So I decide to run git clean to remove them from the working directory. I tried with git clean -f, git clean -n, git clean -i and git clean -d but none of them work, they don't even throw an error, they simply don't do anything. I even tried to remove them manually from the folder but they still appeared untracked.

I think it has something to do with the fact that these three files were conflicted when I pulled dev_alan-es_rich, because there were some commits I did remotely because git push wasn't working. So when the merge conflict appeared, I reset the local commits (that were basically the same I did remotely) with the --soft option (I think that was my mistake) and then pulled.

I would appreciate your help with this problem. I was thinking I could commit them and then reset them with --hard, but I would really like your opinion first.

Thanks and sorry for the inconveniences.

tajmone commented 2 years ago

@Rich15, unfortunately the time difference between Italy and Venezuela makes it hard for us to be online at the same time — in Italy the clock is six hours ahead, so we only manage real time communication when I'm working at late night (yesterday I felt asleep early and wasn't able to catch up with your reply).

Hence, I'll try to pack as much info as possible in each response, hoping it can lead to an immediate solution. I hope this info will help you solve at least the problem of how to keeping your local and GH forks in synch with this upstream repository.

On Rebasing

Understanding Git rebasing operations — especially how to solve conflicts — was one of the hardest learning aspects for me, when I started using Git. What really helped me was using a GUI frontend for Git, which would always show me the Git graph of the branches (both local and remote), which really helped me grasp where the various commits were, via a visual representation of their state.

This helped me not only in the learning stage, but I still rely on the graph during work sessions, to constantly peek at the repository status. I use Sublime Merge to handle all the Git work, although some operations I still carry them out via Bash, which is more practical in many cases (e.g. piping between Git commands and other tools).

What do you use to work with Git? Just the plain Git command line? or do you use some Git GUI frontend? or you have an editor with integrated Git (e.g. VSCode, Atom, Sublime Text)?

The native Git package does contain a GUI tool, you can always open that up to see the repository graph in a neat graphical representation (really helps).

I haven't yet had the time to add a CONTRIBUTING document to this repository, but you can refer to the CONTRIBUTING.md which I wrote for another repository (completely unrelated to ALAN):

Of course, that being another repository the mentioned branches names are different, but the principles (and strategies) are basically the same.

And there's always the Git book I'm translating — the link has now changed, since it's publicly available on its own website now:

Problem with Rebase

I've now added to my local clone your fork as a remote, so I can fetch from your fork to see its status locally, and pull your branches if I need to (e.g. to create backups, test them, etc).

NOTE — I've checkout a copy of your dev_alan-es_rich branch locally, and also created a backup branch of it too, just in case things go bad while your try to fix things. Usually it's rare to end up loosing contents with Git without receiving warning messages, but you never know. In any case, I have a copy of your changes, so your work is safe and you don't have to worry about loosing it.

Check Remotes Configuration

Just to make sure your setup is correct, can you confirm me that your local repository has both the origin remote (pointing to your fork) and the upstream remote (pointing to this "original" repository)?

If in doubt, open the Bash and type

$ git remote -v

You should get something like:

origin  https://github.com/Rich15/alan-i18n (fetch)
origin  https://github.com/Rich15/alan-i18n (push)
upstream  https://github.com/alan-if/alan-i18n (fetch)
upstream  https://github.com/alan-if/alan-i18n (push)

(if you're using SSH instead of HTTP the URLs will differ)

If you don't see the upstream remote, add it via:

$ git remote add upstream https://github.com/alan-if/alan-i18n

(that's for the HTPP protocol, if you use SSH tweak the URL accordingly)

You need to upstream remote to keep your fork in synch with this repository.

NOTE — The names origin and upstram are merely conventions (they have no special meaning in Git, except origin being the default remote assigned by GitHub). The origin branch always point to your GitHub repository, whereas upstram points to the "original" repository of a forked repository — for me and @thoni56, this repository is just origin since we're not working on a fork.

Update Your Local & Fork Repos

I can see that you fork is still lagging behind our upstream.

You must always ensure that your local clone is in synch with the upstream, at least for the branches you are interested in:

The other branches you don't need to worry about, unless you're interested in following ongoing development for English, Italian, Swedish, etc., before it gets merged into main.

TIP! — I advise you to read this article on the GitHub Workflow, so you get a clear picture of how to contribute via a fork:

In order to get updated info on the status of the upstream repository, open Bash inside your local fork directory ant type:

$ git fetch upstream

this won't change anything in your repository, it will just obtain info on the status of the upstream, so it's always a safe operation to carry out.

In order to update the main branch in your local fork:

$ git checkout main
$ git pull upstream/main

after you've done this, your local main branch will mirror (i.e. be in sync) with the upstream repository.

This of course requires that you've defined the upstream remote, pointing to this repository (as mentioned above).

Also, git checkout and git merge/rebase (and any contents altering commands) won't work if you have unstaged files in the work area! (see below)

You should then update your fork on GitHub too:

$ git push

Similar steps need to be done for the dev_alan-es branch too:

$ git checkout dev_alan-es
$ git pull upstream/dev_alan-es
$ git push

After this is done, you now need to rebase your development branch (aka the pull request branch) to ensure it's based on the latest upstream base development branch:

$ git checkout dev_alan-es_rich
$ git rebase dev_alan-es
$ git push

If you're lucky, you won't have any conflicts to resolve. If there are some, we'll address them later one, but for now, let's focus on updating the status of your local fork.

Unstaged Files Problem

Now I'm having a problem with the rebase part. It says I can't rebase because I have unstaged changes;

Stashing is the solution (so you can the pop them again later):

I run git status and confirm there are three untracked files. First I try to stash them with git stash but when I check again they're still there. I add the --include-untracked but nothing changes.

Mhhh. That's strange. You should check whether Git has created a stash commit in your fork (a stash is just a special commit which is handled by Git, from which you can retrieve the stashed file, either by retrieving them but keeping a stashed copy or retrieve them and delete the stash commit).

Windows and Git: Handles Preventing File Deletions

What really matters here is understanding if you have already committed your changes to your own dev branch. Unstaged files often popup during branch checkout operations, e.g. because the .gitignore file was updated in one branch but not the other, so some ignored files show up when you switch to the branch where the .gitignore file isn't updated.

Also, Windows is known to cause similar problems due to running applications having an handle on open files (often even files that were opened and closed), which interfere with Git. Example, you've edited a file in your editor, for your dev branch work, and then you git checkout main. When switching branches, Git will try to delete all files which are not present in that branch, but some application(s) might prevent this by claiming a handle on that file because they are (or have been) using it, so Git will fail to delete them and they will show up in your work areas as unstaged filed!

The same is true with git stash, were Git might create the stash commit correctly, but fail to delete the files after having stashed them.

Usually you should see an error message reporting the failure to delete these files, but that largely depends on whether you're using Git command line or some GUI tool or editor/IDE instead.

Since you're the only one who can see these untracked files, you'll have to inspect them and decide whether you can safely delete them or not. By looking at the filenames, you should be able to easily detect if these are files carried over from the previous branch, due to the above mentioned problem.

In any case, if you know for sure that you've committed all changes in your dev branch, it should be safe to always delete those untracked files.

To avoid these problems, I always edit files with Sublime Text and Sublime Merge, which never create these handles problems. I also don't use Window's File Explorer (which is a major cause of these handle problems) but use Altap Salamander instead (once a commercial tool, now free, but unfortunately no longer updated, and doesn't support Unicode).

Rich15 commented 2 years ago

Git tools

What do you use to work with Git? Just the plain Git command line? or do you use some Git GUI frontend? or you have an editor with integrated Git (e.g. VSCode, Atom, Sublime Text)?

I mostly use Git Bash or Powershell for Git operations, but sometimes I use the VSCode terminal for staging and committing, as well for pushing. I don't use any GUI for Git, but I'll try one now.

Repo Setup

Upstream

Just to make sure your setup is correct, can you confirm me that your local repository has both the origin remote (pointing to your fork) and the upstream remote (pointing to this "original" repository)?

Yes, I have both the upstream remote and the origin remote. I also synched the dev_alan-es branch with the upstream one. I also pulled changes but I didn't rebase my branch (as you've already seen, since we're talking about it).

Unstaged files

Mhhh. That's strange. You should check whether Git has created a stash commit in your fork

Running git stash does create a stash (showing no error) but doesn't delete the files. git clean doesn't do anything either.

My commit situation

The problem I had was with git push. When pushing, it told me I had to update .NET to a newer version (>= 4.7.2). However, first commits were working well and pushed to the remote even without downloading .NET framework update.

When I tried to push my last 2 commits, it asked me for authentication from Github, but didn't let me write my password nor my PAT (the username was working though). I also tried to do it from the VSCode terminal but didn't work either.

I downloaded the .NET framework installer, but it stopped everytime because "the timestamp signature and/or certificate could not be verified". Apparently it has something to do with my Windows version (I have to update my OS).

Finally I decided to upload the files directly in Github and commit them.

When I pulled from the remote, now I had two identical commits, and the auto-merge failed, so I used git reset HEAD~2 --soft to eliminate the commits and keep the files. The merge conflict was resolved and my local branch was up to date.

Here is when the rebase problem with dev_alan-es appeared.

Deleting files

In any case, if you know for sure that you've committed all changes in your dev branch, it should be safe to always delete those untracked files.

Since the changes are already on the remote, I think deleting the files should be no problem (You have a backup of my branch and I could make a backup of them too). However, seeing that none of the usual ways is working (i.e. git stash, git clean and deleting them manually), I think I would have to commit them and then reset them --hard and see if that works.

I'll be busy the rest of the day so I don't think I would be able to get in the computer (In fact, I'm writing this from my phone). But tomorrow I'll try this option and see if that solves it.

Thanks @tajmone for all the advices. I'll take a look at all the links and documentation you have provided (thanks for updating the link for Das Git-Buch btw). Tomorrow I will update the changes and whether the problem was solved (if the PR updates, probably it was solved).

tajmone commented 2 years ago

Unstaged files

Running git stash does create a stash (showing no error) but doesn't delete the files. git clean doesn't do anything either.

That's very strange, according to the git-stash documentation (emph. added):

Use git stash when you want to record the current state of the working directory and the index, but want to go back to a clean working directory. The command saves your local modifications away and reverts the working directory to match the HEAD commit.

The problem might be due to a handle that prevents deletion. I remember facing that a lot when I used to work with Git initially, and still face this problem when File Explorer is open.

But then, Win 10 is getting a lot of updates to solve conflicts with Git (including the new feature for case-sensitive directory trees), since MS acquired GitHub. Older versions of Win won't be enjoying these updates.

My commit situation

The problem I had was with git push. When pushing, it told me I had to update .NET to a newer version (>= 4.7.2). However, first commits were working well and pushed to the remote even without downloading .NET framework update.

Git doesn't use .NET, this might be something required for the new credentials manager by MS. You can always tweak Git settings to use its own credentials manager — IRC, the Windows credential manager in the Git for Windows package was added fairly recently, so if you had installed Git before that it might have fallen back on the native Win credential manager, so you'll have to fix that manually.

I downloaded the .NET framework installer, but it stopped everytime because "the timestamp signature and/or certificate could not be verified". Apparently it has something to do with my Windows version (I have to update my OS).

I remember that there's a problem updating .NET v4.x under older Windows of Windows (e.g. when you install the Win SDK), and that you need to manually uninstall the default .NET v4 for the update to work. I had to do that many times to install the Win SDK. Since the .NET update will reinstall all the .NET v4 required tools, you won't loose anything. But you can't update it without uninstalling the older one, due to some problem with the original version 4 distribution.

Finally I decided to upload the files directly in Github and commit them.

The WebUI has some limitations though, it's always preferable to handle all Git operations locally.

The merge conflict was resolved and my local branch was up to date.

You should then push your fixed branches to origin, otherwise I won't be able to see locally the status of you fork — I've fetched from Rich15/alan-i18n, but dev_alan-es is still behind the upstream.

If you wanted I could solve the problem locally, i.e. rebase your PR branch myself and then merge it into the repository, but it makes more sense if I help you doing this, since rebasing is one of the most common operations during development work (much more than merging). In any case, I'll try the operation locally, just to see how many file conflicts (if any) are waiting ahead.

I can always fix the branch and the create a pull request on your fork, targetting your PR branch, so that these changes will end up in this PR when you force push the fixed branch.

I'll be busy the rest of the day so I don't think I would be able to get in the computer

Don't worry, no hurry. I just hope you'll solve these problems, since I know how frustrating can be learning Git, anyway it's worth it since Git is a very useful tool (as useful as it's ugly and unfriendly). I'm a bit worried about those files not being deleted, and I'm starting to wonder whether there might be some app running in the background that interferes (e.g. antivirus, anti-malware, and other apps that monitor files).

I guess that you're using Win7 32-bit because you have a 32-bit motherboard. Unfortunately many Linux distros don't support 32-bit releases anymore, otherwise I would advised you to switch to Linux, since Win 7 is getting old and compatibility problems will only grow in the future (especially with Win 11 coming out in Winter).

tajmone commented 2 years ago

It's a Git Bug!

I've tried rebasing locally, and I get the same problem as you do, and was puzzled: every time I clean the branch (delete any files in the work area) when I attempt git rebase these files come up again...

I then remembered that I came across this Issue before, on another repository, and that I had managed to isolate the problem and fix it:

https://github.com/git-for-windows/git/issues/694#issuecomment-826219425

So it's a bug specific to Git for Windows. Basically, your commits are sending Git for Windows nuts due to either EOL inconsistencies or (and) file permissions differences, which Git Win can't handle well — possibly this is due to the fact that you've committed them via the GH WebUI, which runs on Linux and might have enforce some defaults which don't match the expected ones (which is why I don't like the WebUI).

I thought this bug had been fixed in the meantime, but that's not so.

On top of that, your local copy of the Spanish dev branch was out of synch because I had amended it and force pushed it in the meantime, which is why this rebase is so complicate to handle.

I'll try fix the problem locally and create a PR on your fork with the fixed branch, so you can reset your PR branch to it and just force push.

tajmone commented 2 years ago

New Branch With Fixes

I've created a new branch fixRichPR:

https://github.com/alan-if/alan-i18n/tree/fixRichPR

This branch should recreate your PR branch dev_alan-es_rich — I've cherry picked all your commits into an updated copy of dev_alan-es, and there was only a single commit that was creating trouble.

Could you please check that there are all the commits of your original PR branch?

You can checkout it out locally, to compare it with your original PR branch.

If the fixed branch looks OK, you only have to hard reset dev_alan-es_rich to it:

git checkout `dev_alan-es_rich`
git reset --hard upstream/fixRichPR
git push -f

then this PR will be replaced by the contents of fixRichPR, which are in synch. All commits are still in your name, even if I had to resolve conflicts.

PS: I couldn't manage to create a PR on your repository, because it's a fork (if mine was a fork too I could).

Rich15 commented 2 years ago

Solved (I think)

Hi @tajmone. I was busy this last two days, but I think the problem is solved. Finally the conflicted files are gone and I could force push my branch with the fixRichPR changes. That bug confused me a lot, and for a moment I thought I had broken something. Even when I did checkout to fixRichPR there was still a "phantom" untracked file appearing in the Command Line.

Thanks so much for all your help with this, for the fix branch and for all the advices. I'm slowly getting more comfortable with Git, and from now on I will remember to pull and rebase! I've also downloaded a Git GUI (GitKraken) to make things easier and to avoid these kind of bugs in the future (including that I'm unable to push from the Command Line for some reason). I will try to change the Git credentials so that .NET alert stops popping everytime I do a push.

About Failed Builds

Now I see that there are two failed Builds in my branch. I'll run it locally to see what's going on. I understand I should install EClint with npm first and then run the test, right? When I see what's wrong I'll fix the files and push them, hopefully without so much trouble this time.

Thanks again for everything!

tajmone commented 2 years ago

Finally the conflicted files are gone and I could force push my branch with the fixRichPR changes.

Excellent! This is the ideal working condition for dev branches, so you can always rebase onto the reference branch (while working or with PRs). Keeping your work branches always up to date while working locally, by fetching upstream and rebasing (if need to) is always good practice.

Pull requests can often be merged even if the incoming PR branch is out of synch with the target branch, as long as the PR commits don't affect any files which were changed in the base branch commits beyond the branching point — i.e. commits that were added after you branched off the base branch.

Essentially, rebasing means:

  1. Updating the branch by integrating the missing commit (i.e. the commits added after you created your own branch)
  2. re-playing your changes (i.e. your commits) on the updated branch.

Git attempts the operation, and if there are no conflicts it becomes effective (note that commit hashes are re-created from scratch, because Git is actually re-creating your commits on the new branch HEAD, so even if the have the same title, message, author and other metadata, they are different commits now).

If there are conflicts (i.e. both branches now changing the same files), Git will ask you to resolve those conflicts via a three way merge, one file at the time, for every commit that needs to be replayed (which is why it's better to rebase often, so in case of conflicts there are fewer steps to handle). A three way merge is basically choosing which edits to keep in the new (i.e. rebased) commit, selecting edit chunks ("hunks") from either branch (the target vs your branch). Sometimes solving conflicts can be very entangled and time consuming. Other times it's easy.

The Git Bug

That bug confused me a lot, and for a moment I thought I had broken something. Even when I did checkout to fixRichPR there was still a "phantom" untracked file appearing in the Command Line.

I know, that bug is totally disruptive, it simply blocks all operations and require horrible workarounds to make Git usable again on the repository — and even if you fix it by amending the HEAD commit of the branch, it can still pop-up when checkout previous commits that affect end-of-lines or file permissions (and possibly other file attributes), which means that it can still affect operations like rebasing or cherry picking, since they involve Git checking out those commits.

When this bug shows up, the best solution is trying to amend every commit in that branch, to ensure there are no "invisible" differences (i.e. changes that don't show up in the editor), but that's a tricky operation to do, so the best thing is trying to squash as many commits as possible into single commits, to reduce their number, and fix all EOLs and permissions while doing so.

Thanks so much for all your help with this, for the fix branch and for all the advices. I'm slowly getting more comfortable with Git, and from now on I will remember to pull and rebase!

Trust me, we've all been there! And when someone tells you that learning Git was a smooth experience, probably he/she's not being since, because Git is hard to learn. Probably, since you've started off encountering the bug, you've already banged your head against the "hard way", which has forced you to grasp all the internal problems of rebasing, which is an important step toward that Eureka! moment were everything falls in place and one glimpses the overall scheme of Git things, understanding how repository graphs work.

I've also downloaded a Git GUI (GitKraken) to make things easier and to avoid these kind of bugs in the future

That's a good GUI.

(including that I'm unable to push from the Command Line for some reason). I will try to change the Git credentials so that .NET alert stops popping everytime I do a push.

Credentials Manager

I suspect these are both related to the credentials manager. Recently GitHub has stopped allowing to use the GH account password for Git operations, requiring the PAT instead (Private Access Token), which means that many Git operations will now fail. You'll probably need to reset the stored credentials in order to use the PAT. Since Git CLI and the various Git GUIs (or editors) might be using different Git settings (or even different Git binaries, if they ships with a custom Git package), you'll have to find out which credentials manager each tool is using, and possibly ensure they all use the same manager.

If the .NET update is because of the credentials manager, it means you're using the MS manager (which of course you should also try to update), if this is the case you should try to switch to Git's native manager.

You can check your Git settings with

$ git config --global credential.helper

See also:

I advise you to search the GitHub Community forums for a solution to this, since credentials problems is a topic that comes up very often, and if you can't find a solution you can always ask again. Support in the Community is almost in real time.

About Failed Builds

Now I see that there are two failed Builds in my branch. I'll run it locally to see what's going on.

Currently "failure" just means that there are code styling issues, like trailing spaces, missing final empty line, etc. So it's not really a huge deal. In the future, the build might verify things like compilation, documents conversion, etc., but right now it's just about code consistency.

I understand I should install EClint with npm first and then run the test, right? When I see what's wrong I'll fix the files and push them, hopefully without so much trouble this time.

That's right. Just know that EClint is a buggy tool, no longer maintained, so it's not fully reliable (especially on Windows). We're waiting for a bug fix in another tool to replace it, but have no idea how long the bug fix will take.

If you are using an editor that is EditorConfig compliant, you shouldn't even need EClint, since the editor should be complying to the .editorconfig settings. You should check at the link below whether your editor(s) and IDE(s) support EditorConfig natively or whether you need to install a plug-in/extension:

In any case, these "failures" can be taken more as warnings, so it doesn't mean the PR can't be merged. The code can be reformatted correctly later on, and the build will then pass from that point on.

The important thing is ensuring that line ends (EOLs) are consistent within the file (i.e. not mixing LF and CRLF), but that should be already enforce by the .gitattributes, unless the contributors hasn't enabled the Git options to strictly enforce these.

By all means, avoid committing changes via the GitHub WebUI, especially in branches where you have already started some work.

The reason we want to enforce consistent code styles is to avoid seeing "noise" in the commits, e.g. because an editor changes all indentations from spaces to tabs when the user saves the file, which would then show up as the entire file having been edited, whereas the meaningful code changes only involves a few lines. See:

IMPORTANT — I noticed that some of your commit messages contain CRLF EOLs, which I guess can only happen if you are using Git under CMD or PowerShell. Please, always use Git Bash for all Git operations, many CMD tools can't even handle UTF-8 properly, and PowerShell won't handle correctly EOLs, mixing CRLF and LF — which is likely to trigger that nasty Git bug.

Merging the PR

Before merging the PR, I just want to inspect the individual commits, to ensure that there's nothing left there that might bring that nasty bug back. I'll probably to that tonight, when I have more time.

Also, I was considering that some commits could be squashed together as a single feature (e.g. all new and fixed sources translations in one commit, your new test adventure in another); doing so also makes it easier to amend the code styles that are currently causing EClint to complaint.

So, I might be amending the fixRichPR branch, squashing commits together and/or fixing code styles so that, if the amended branch is OK with you, you can hard reset the commit branch against it, again, and we end up with a cleaner PR. In any case, I'll buzz you when it's time.

tajmone commented 2 years ago

Solutions, Accented Letters & Prompt

I noticed that in your .a3s solution files you've included the prompt before the actual command, e.g.

> x pantalón

But you should just type the command, without prompt, on each line:

x pantalón

(also, the prompt could be defined as something else than > in some adventures)

In a commit message you noted:

Note: There is a problem running 'pantalón' as an input in the adventure, apparently because of the accent mark

No, it's because of the prompt. I've amended the commit locally, removing the prompt (and the error note) and it now works fine:

> toma pantalón
Tomas el pantalón.
tajmone commented 2 years ago

Squashed all Commits into One

@Rich15, I've amended your PR branch in fixRichPR branch:

https://github.com/alan-if/alan-i18n/tree/fixRichPR

I've squashed all your commits into one, since they are all changes belonging to a same set of features, and fixed the code styles issues that EClint was complaining about (trailing spaces, and a couple of missing empty lines).

I tried to keep two commits, one for the translation fixes and another for the new test adventure, but I didn't manage to due to some problems (one of the commits would have resulted in an unstable state).

Also, I hope you don't mind that I took liberty to do some fixes:

You still result as the author of the commit, since I only amended the original commits.

Amending PR Branch & Merging

So, it you're OK with my changes, just reset this commit branch to fixRichPR, like you did before:

git checkout `dev_alan-es_rich`
git reset --hard upstream/fixRichPR
git push -f

then we should be good to merge the PR.

Thanks a lot for your contributions, they're very precious. So much so that I'll be deleting my previous test adventure for wearables, replacing it with your instead (no need to have too). I'll do that after the merge, and also update the CHANGELOG, etc. and then see if we're ready to create a new Spanish Lib version (merging it into main).

Rich15 commented 2 years ago

Reset and push

The changes have been integrated and the PR is now updated!

Credentials

Recently GitHub has stopped allowing to use the GH account password for Git operations, requiring the PAT instead (Private Access Token), which means that many Git operations will now fail.

I created my PAT, and when I tried to push from Powershell or Git Bash, I wrote my username, but then it didn't let me paste the PAT, and even more, it didn't let me write anything in the "password" field. I don't know if this could also be related to credentials but I checked and I'm using the Windows Manager Core (which from what I understand it's the latest version). I'll continue to push from GitKraken for now, although I'll consider changing to Git's native credential manager and see if anybody has experienced the same problem as mine.

Code styles and the bug

The reason we want to enforce consistent code styles is to avoid seeing "noise" in the commits, e.g. because an editor changes all indentations from spaces to tabs when the user saves the file, which would then show up as the entire file having been edited, whereas the meaningful code changes only involves a few lines.

I noticed this when looking at one of the conflicted untracked files. When I looked up what was modified it appeared like the whole file changed, but there were just some extra spaces at the end of lines that were apparently removed. However I couldn't discard the changes nor stash them (as you saw) so I don't know if the code styles had something to do with it.

Afortunately VSCode supports .editorconfig natively, so I don't need to install any plugin. I'll use only Git Bash and GitKraken from now on to avoid encountering this bug again.

Fix and squash

Also, I hope you don't mind that I took liberty to do some fixes:

  • In the .a3s script of your new adventure:

    • Remove the prompt preceding the commands (now it works).
    • Reduced multiple empty lines to a single empty line, (to prevent excessive spacing).

  • Added "pantalon" as an alternative name for "pantalón" [...]

  • Tweak the commit message to a briefer and more generic one.

Thanks for that! Now I understand why it didn't work. For future tests/adventures I'll add an alternative name for objects with accent marks.

Thanks a lot for your contributions, they're very precious. So much so that I'll be deleting my previous test adventure for wearables, replacing it with your instead (no need to have too). I'll do that after the merge, and also update the CHANGELOG, etc. and then see if we're ready to create a new Spanish Lib version (merging it into main).

It's a pleasure, thanks to you for taking time to review and fix them, and for helping me with all the Git stuff.

I think the next step would be to handle the gender of actors properly, and maybe create the grammar initialization module. I'll soon try and tweak some things locally to see if I find a way to solve the actors issue.

tajmone commented 2 years ago

Credentials

I created my PAT, and when I tried to push from Powershell or Git Bash, I wrote my username, but then it didn't let me paste the PAT, and even more, it didn't let me write anything in the "password" field. I don't know if this could also be related to credentials

Definitely a problem with the credentials manager

... and see if anybody has experienced the same problem as mine.

Lots of people on GitHub Community are reporting similar problems (on all OSs) since GH stopped allowing using the password for Git operations. In most cases, the problem is just that people don't know how to remove the old cached credentials, so they can use the new PAT — i.e. because it was such a long time ago that they had set up Git that they simply can't remember the details. But in many cases there are also complications due to different tools using different Git configurations (or even different Git packages).

So it's definitely worth looking up this on GH Community, since it has been discussed many times over, with solutions covering different credentials managers, editors, Git GUI fontends, etc.

but I checked and I'm using the Windows Manager Core (which from what I understand it's the latest version). I'll continue to push from GitKraken for now, although I'll consider changing to Git's native credential manager

That's good, I wasn't sure if you had the old (now deprecated and no longer updated) "Git Credential Manager for Windows" (GCM).

From Git Credential Manager Core repository (emphasis added):

Git Credential Manager Core (GCM Core) is a secure Git credential helper built on .NET that runs on Windows, macOS, and Linux.

Compared to Git's built-in credential helpers (Windows: wincred, macOS: osxkeychain, Linux: gnome-keyring/libsecret) which provides single-factor authentication support working on any HTTP-enabled Git repository, GCM Core provides multi-factor authentication support for Azure DevOps, Azure DevOps Server (formerly Team Foundation Server), GitHub, and Bitbucket.

Git Credential Manager Core (GCM Core) replaces the .NET Framework-based Git Credential Manager for Windows (GCM), and the Java-based Git Credential Manager for Mac and Linux (Java GCM), providing a consistent authentication experience across all platforms.

As you can see, it does cover a more ample range of credentials, so if you can make it work it's worth using it. The downside is that its a .NET app, which is causing you all those update problems (since Win7 isn't getting all the Win10 updates, I'm not sure how this will work out in the long run, especially for 32-bit).

Code styles and the bug

I noticed this when looking at one of the conflicted untracked files. When I looked up what was modified it appeared like the whole file changed, but there were just some extra spaces at the end of lines that were apparently removed.

Even if a line ending is changed from CRLF to LF Git will see it as a file change (although not visible in the the editor and most diffing tools). So, if a file is opened and saved with an editor that blindly enforces a specific EOL this could result in the whole file showing up as modified.

All these added/removed trailing spaces and EOL differences make it hard to look at PRs' and commits' diff and see what meaningful changed they bring (hence the term "noise").

However I couldn't discard the changes nor stash them (as you saw) so I don't know if the code styles had something to do with it.

That Git for Windows bug is hard to pin down, it seems to be affected by any changes that are not directly part of a file, like permission and EOLs (because Git normalizes EOL at check-in/out). E.g. if I change permissions of a file and then commit the change, Git seems unable to handle branch switching, rebasing, and all sort of operations involving commits checkouts, because the the affected file keeps ending up in the work area, even if you delete it.

Another source of the problem is when a file only differs in EOLs, or even worst when a wrong EOLs get into the file — i.e. because a user hasn't enabled the Git options to enforce EOL consistency at staging time, and proper CRLF normalization on Win OS.

I'm not sure what happens internally to Git, but my guess is that there are some discrepancies in the code that handles comparisons of files in the Git index vs the work space and stage. Since Windows doesn't have the same file permissions system as Linux/Mac, permission bits/modes are mostly emulated, so even if they are wrongly set they go by unnoticed (e.g. a Bash script executes even if it doesn't have the executable bits set).

Alt Names for Accented Letters

For future tests/adventures I'll add an alternative name for objects with accent marks.

Although I've added it there, I'm now not entirely convinced it's needed (I mean, these accents are very common, it could mean a lot of extra work). The test adventures are mostly used in automated tests anyhow, so it's not as if end users will actually be typing them, and in worst case scenario they could just paste the accented noun.

Maybe in a real adventure it's worth adding the synonyms and alternative names without accents (maybe the player is Spanish, but is playing on the PC of someone else who has a US keyboard, with no accented letters?), but I'm not sure we need that in the tests.

It's the same thing in Italian, since we use a lot of accented letters too. But in Italian games I always add alternative nouns without the accents, as well as with different accents (acute vs grave) because many Italians today don't know any longer the difference between the acute and grave accent, and when to use either (or none). It's one of those grammar rules that have been partly dropped.

So, you'll probably know better than me whether in Spanish it's required (or worth) adding the alternative nouns without accents — or maybe doing so for some words but not others, etc.

GNA & New Grammar Module

I think the next step would be to handle the gender of actors properly, and maybe create the grammar initialization module.

Definitely. And I noticed your comments pointing out where in the code we need to add number checks and where it's not needed (very useful annotations). Now that we start to have better tests coverage we can definitely begin working on the separate grammar module — I suggest creating a dedicate dev branch for this, e.g. dev_alan-es-gramatica (no accents!), because it's going to require thorough checking before merging, so it's going to be a long standing branch and it shouldn't block development of other Spanish features.

So dev_alan-es remains the base branch for all Spanish work (all PRs should go there), but for the grammar module we'll have a "secondary base branch" dev_alan-es-gramatica, which we'll be constantly rebasing on main to integrate all released changes while we work at it. In other words, the work on the grammar module shouldn't affect in any way the ordinary work on the Spanish library (translation, code fixes), and vice-versa; after all, we have no guarantees that this module will actually work out, so there's always the possibility that the branch gets discarded in the end.

But the main reason why I want to keep its work in a separate branch (and have it track main instead of dev_alan-es) is because the new grammar module will require tweaking all the ALAN sources of the test adventures, Vampiro, etc. — more than once! If we adopt the article-based initialization approach, it's going to change radically how game entities are defined in the ALAN sources.

I'll soon try and tweak some things locally to see if I find a way to solve the actors issue.

Great!

Even in English it can be harder than it looks to come up with examples covering edge-cases, but in languages like Spanish and Italian (which add gender to the problem) it's even harder, because we have different articles too, and words with special ending being treated differently, etc.