I wrote a parser for Goethe-Gymnasium Hamburg (189802). For that, I had to change the following in general files:
Added Goethe parser to Utility.java
Comparing Class/Teacher names case insensitive in Reader.java (required by Goethe's timetables)
I wrote a parser for Goethe-Gymnasium Hamburg (`189802`). For that, I had to change the following in general files:
* Added Goethe parser to `Utility.java`
* Comparing Class/Teacher names case insensitive in `Reader.java` (required by Goethe's timetables)
Having classInClass case-sensitive improved accuracy by a little when filtering info-only entries (especially for schools where the class letter is capital). Could you explain a little more why this change is necessary?
Looks like merging this would mess with the changes of #18. Rebasing your essentials branch onto the goethe branch could probably fix this (I'm not really sure though).
Thank you a lot for your effort!
Having `classInClass` case-sensitive improved accuracy by a little when filtering info-only entries (especially for schools where the class letter is capital). Could you explain a little more why this change is necessary?
Looks like merging this would mess with the changes of #18. Rebasing your `essentials` branch onto the `goethe` branch could probably fix this (I'm not really sure though).
In Goethe's substitute tables, the classes letters are sometimes upper case and sometimes lowercase. Same for teacher's names.
The merge conflict seems to be fixed now...
Btw. How is your release management? If you approve this pull, is it released automatically or does it take some time?
In Goethe's substitute tables, the classes letters are sometimes upper case and sometimes lowercase. Same for teacher's names.
The merge conflict seems to be fixed now...
Btw. How is your release management? If you approve this pull, is it released automatically or does it take some time?
Furthermore, there exists a String resource, error_parsing_partial, and it is used as info value when a whole page fails (illogically, then, even if it is the only page…). Maybe the wording can be changed from "Some data could not be parsed" to account for your case where only a single entry fails, but I'm not sure to what.
In case displaying an entry fails, you are currently [initializing an entry with empty Strings][1]. I'd prefer it if you could pass `null` instead as [all `Entry()` parameters are annotated as `@Nullable`][2].
Furthermore, there exists a String resource, [`error_parsing_partial`][4], and it is used as `info` value [when a whole page fails][3] (illogically, then, even if it is the only page…). Maybe the wording can be changed from "`Some data could not be parsed`" to account for your case where only a single entry fails, but I'm not sure to what.
[1]: https://notabug.org/fynngodau/DSBDirect/pulls/19/files#diff-66b90243f48bac77696d6c496124b7528da107fR125
[2]: https://notabug.org/fynngodau/DSBDirect/src/ca6d96f167b824249076fb0ee6ad3947077b8b9d/app/src/main/java/godau/fynn/dsbdirect/table/Entry.java#L36
[3]: https://notabug.org/fynngodau/DSBDirect/src/master/app/src/main/java/godau/fynn/dsbdirect/activity/MainActivity.java#L508
[4]: https://notabug.org/fynngodau/DSBDirect/src/ca6d96f167b824249076fb0ee6ad3947077b8b9d/app/src/main/res/values/strings.xml#L41
Release is whenever I say so. This can gladly be after this pull request is merged. F-Droid propagation would take some time as usual, and Play releases are also not pushed immediately.
It is quite the manual process, so I usually wait until a few fixes have accumulated.
Release is whenever I say so. This can gladly be after this pull request is merged. F-Droid propagation would take some time as usual, and Play releases are also not pushed immediately.
It is quite the manual process, so I usually wait until a few fixes have accumulated.
I don't understand why to change error_parsing_partial. That's actually what happened in that case: Only some parts failed to load.
Okay, I'm now passing `null`.
I don't understand why to change `error_parsing_partial`. That's actually what happened in that case: Only some parts failed to load.
Concerning case sensitivity: since the gain in accuracy is minimal anyway, it's probably not worth it to differentiate based on what parser has parsed the plan here. A more effective filter for info entries is needed anyway. Tracking that effort in #21.
Concerning case sensitivity: since the gain in accuracy is minimal anyway, it's probably not worth it to differentiate based on what parser has parsed the plan here. A more effective filter for info entries is needed anyway. Tracking that effort in #21.
Thanks for little change again. I was thinking, in case a lot of entries fail to parse in a row, it would make more sense to say something like "We're missing an entry here" many times instead of "some data", but nevermind. If it's broken, it's going to look broken either way.
Thanks for little change again. I was thinking, in case a lot of entries fail to parse in a row, it would make more sense to say something like "We're missing an entry here" many times instead of "some data", but nevermind. If it's broken, it's going to look broken either way.
If you open the preferences screen, you'll notice that the Parser preference has been blocked because There is no choice. I'll simply revert the commit that added this (c81b3a3023) after merging this pull request.
If you open the preferences screen, you'll notice that the Parser preference has been blocked because There is no choice. I'll simply revert the commit that added this ([`c81b3a3023`][commit]) after merging this pull request.
[commit]: https://notabug.org/fynngodau/DSBDirect/commit/c81b3a302348220dc8920fa7f9a59e8de62a31f9
In order for the preference in the preference screen to allow selecting Goethe manually, you need to add it to the two String array resources here. The Röntgen option should be removed, but I forgot to do that. If I do it now, it will only cause a merge conflict, so I'm asking you to simply replace it with Goethe.
In order for the preference in the preference screen to allow selecting Goethe manually, you need to add it to the two String array resources [here][here]. The Röntgen option should be removed, but I forgot to do that. If I do it now, it will only cause a merge conflict, so I'm asking you to simply replace it with Goethe.
[here]: https://notabug.org/fynngodau/DSBDirect/src/master/app/src/main/res/values/strings.xml#L209
I see you're trying to fit the subject into the lesson field ((\d.* Std\. \w+)). However, the lesson field is intended to be very small, usually only one number, though it can of course also hold a duration (1-10).
1.-5.Std.Musik is unfortunately way too long and breaks the cards layout as seen in the attachment. I think Std. is redundant – it already is in the lesson column – and every . can be omitted to improve readability. The subject should go somewhere else.
In the Untis parser, I am concatenating class and subject using a · character. This may not be a wise choice for you as well, as space for class is very limited in list view (the Untis plan I've seen had lesson abbreviated). I think it would make more sense for you to move the subject to the info field. I imagine the parser to make this
8B | 1. - 2. Std. Sport bei MAN im Raum ESH fällt aus
become this (affectedClass | lesson | replacementTeacher | info):
8B | 1-2 | MAN | Sport im Raum ESH fällt aus
I see you're trying to fit the subject into the lesson field (`(\d.* Std\. \w+)`). However, the lesson field is intended to be very small, usually only one number, though it can of course also hold a duration (`1-10`).
`1.-5.Std.Musik` is unfortunately way too long and breaks the cards layout as seen in the attachment. I think `Std.` is redundant – it already is in the lesson column – and every `.` can be omitted to improve readability. The subject should go somewhere else.
In the Untis parser, I am [concatenating class and subject using a `·` character][concat]. This may not be a wise choice for you as well, as space for class is very limited in list view (the Untis plan I've seen had lesson abbreviated). I think it would make more sense for you to move the subject to the info field. I imagine the parser to make this
`8B | 1. - 2. Std. Sport bei MAN im Raum ESH fällt aus`
become this (`affectedClass | lesson | replacementTeacher | info`):
`8B | 1-2 | MAN | Sport im Raum ESH fällt aus`
[concat]: https://notabug.org/fynngodau/DSBDirect/src/ca6d96f167b824249076fb0ee6ad3947077b8b9d/app/src/main/java/godau/fynn/dsbdirect/table/reader/Untis.java#L171
Another thing I've noticed is that your plan is not actually school-specific, but rather generally a HeinekingMedia Vertretungsplan as indicated in the title. I've seen one before searching around on the internet (archive), and it looked very similar. If you can add support for multiple days in one page, we can dynamically detect any HeinekingMedia Vertretungsplan and use the parser for that automatically. But we can finish this pull request first and add more general support and detection at a later date if you prefer.
However, this should be reflected in the UI (the parser selection should show HeinekingMedia instead of Goethe).
Another thing I've noticed is that your plan is not actually school-specific, but rather generally a `HeinekingMedia Vertretungsplan` as indicated in the `title`. [I've seen one before searching around on the internet][before] ([archive][archive]), and it looked very similar. If you can add support for multiple days in one page, we can [dynamically detect][dynamical] any `HeinekingMedia Vertretungsplan` and use the parser for that automatically. But we can finish this pull request first and add more general support and detection at a later date if you prefer.
However, this should be reflected in the UI (the parser selection should show `HeinekingMedia` instead of `Goethe`).
[before]: https://app.dsbcontrol.de/data/5f311cee-3221-4e78-8da5-6ee91e57f604/5827346f-ea1f-4dce-af37-ce9caaadb975/5827346f-ea1f-4dce-af37-ce9caaadb975.html
[archive]: https://web.archive.org/web/20190812201232/https://app.dsbcontrol.de/data/5f311cee-3221-4e78-8da5-6ee91e57f604/5827346f-ea1f-4dce-af37-ce9caaadb975/5827346f-ea1f-4dce-af37-ce9caaadb975.html
[dynamical]: https://notabug.org/fynngodau/DSBDirect/src/ca6d96f167b824249076fb0ee6ad3947077b8b9d/app/src/main/java/godau/fynn/dsbdirect/Utility.java#L128
Okay, I fixed the langth of lessons and moved the subject into the information column. I think I updated the String from Röntgen to Goethe but it anyway allways shows me There is no choice, only Untis boards can be parsed at the moment.
I agree, the general support for HeinekingMedia Vertretungsplan we could improve later.
Btw. I fixed a little bug concerning Android 4 devices in your mailing function.
Okay, I fixed the langth of lessons and moved the subject into the information column. I think I updated the String from `Röntgen` to `Goethe` but it anyway allways shows me `There is no choice, only Untis boards can be parsed at the moment`.
I agree, the general support for `HeinekingMedia Vertretungsplan` we could improve later.
Btw. I fixed a little bug concerning Android 4 devices in your mailing function.
Please also change the German String resource value. I will revert commit c81b3a3023 that causes blocking the preferences after merging.
Your fix doesn't fix it, but thanks for noticing. Here is a patch to fix your fix:
diff --git a/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java b/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java
index 5745af9..2763114 100755
--- a/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java
+++ b/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java
@@ -244,9 +244,7 @@ public class SettingsActivity extends Activity {
findPreference("settings_about_email").setOnPreferenceClickListener(new Preference.OnPreferenceClickListener() {
@Override
public boolean onPreferenceClick(Preference preference) {
- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
- emailTheDev(getContext());
- }
+ emailTheDev(getActivity());
return false;
}
});
* Please also change the German String resource value. I will revert commit [`c81b3a3023`][commit] that causes blocking the preferences after merging.
* Your fix doesn't fix it, but thanks for noticing. Here is a patch to fix your fix:
```
diff --git a/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java b/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java
index 5745af9..2763114 100755
--- a/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java
+++ b/app/src/main/java/godau/fynn/dsbdirect/activity/SettingsActivity.java
@@ -244,9 +244,7 @@ public class SettingsActivity extends Activity {
findPreference("settings_about_email").setOnPreferenceClickListener(new Preference.OnPreferenceClickListener() {
@Override
public boolean onPreferenceClick(Preference preference) {
- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
- emailTheDev(getContext());
- }
+ emailTheDev(getActivity());
return false;
}
});
```
[commit]: https://notabug.org/fynngodau/DSBDirect/commit/c81b3a302348220dc8920fa7f9a59e8de62a31f9
Furthermore, I noticed you are omitting information that starts with the keyword im Raum:
Original: 9A | 6. Std. Religion bei FER im Raum H002 statt bei LIE
Expected result: 9A | 6 | FER | Religion im Raum H002 statt bei LIE
Actual result: 9A | 6. | FER | Religion statt bei LIE
Since it's quite the critical information, please fix this as well.
[Comparing Strings with `!=`][comp] [will not work][so]. You need to use `.equals`:
```
diff --git a/app/src/main/java/godau/fynn/dsbdirect/table/reader/GoetheHamburg.java b/app/src/main/java/godau/fynn/dsbdirect/table/reader/GoetheHamburg.java
index da4462a..6adc157 100755
--- a/app/src/main/java/godau/fynn/dsbdirect/table/reader/GoetheHamburg.java
+++ b/app/src/main/java/godau/fynn/dsbdirect/table/reader/GoetheHamburg.java
@@ -112,7 +112,7 @@ public class GoetheHamburg extends Reader {
String subject="";
pattern = Pattern.compile("\\d.* Std. (\\w+)");
matcher = pattern.matcher(infoText);
- if(matcher.find()&&matcher.group(1)!="bei") subject=matcher.group(1)+" ";
+ if(matcher.find() && !matcher.group(1).equals("bei")) subject=matcher.group(1)+" ";
String info="";
```
Furthermore, I noticed you are omitting information that starts with the keyword `im Raum`:
Original: `9A | 6. Std. Religion bei FER im Raum H002 statt bei LIE`
Expected result: `9A | 6 | FER | Religion im Raum H002 statt bei LIE`
Actual result: `9A | 6. | FER | Religion statt bei LIE`
Since it's quite the critical information, please fix this as well.
[comp]: https://notabug.org/JasMich.de/DSBDirect/src/d57f361e50e6f0803983629708b00aab1123540a/app/src/main/java/godau/fynn/dsbdirect/table/reader/GoetheHamburg.java#L115
[so]: https://stackoverflow.com/a/513935
Original: 9A | 6. Std. Religion bei FER im Raum H002 statt bei LIE
Expected result: 9A | 6 | FER | Religion im Raum H002 statt bei LIE
Actual result: 9A | 6. | FER | Religion statt bei LIE
That's wanted. If the text contains statt bei and a teacher's short code (XXX), it means the room didn't change but only the teacher. (Look at the teacher column: it's unequal to the short code in the info text.) That's why I'm removing the room in that case.
Fixed comparing of the strings.
> Original: 9A | 6. Std. Religion bei FER im Raum H002 statt bei LIE
>
> Expected result: 9A | 6 | FER | Religion im Raum H002 statt bei LIE
>
> Actual result: 9A | 6. | FER | Religion statt bei LIE
That's wanted. If the text contains `statt bei ` and a teacher's short code (`XXX`), it means the room didn't change but only the teacher. (Look at the teacher column: it's unequal to the short code in the info text.) That's why I'm removing the room in that case.
6be17a189f is not really what I suggested, but I suppose it works as well… just emailTheDev(getActivity()); would have sufficed, though, instead of the whole if clause.
To explain why it doesn't matter: emailTheDev() is asking for a Context. Activity extends Context. So getActivity() will be implicitly cast to a Context because that's what is asked for. We could also cast it explicitly with (Context) getActivity(), but it would require more typing.
I didn't realize the omission was intentional… can you be sure that only the teacher changed and that it is not also in another room? It would make sense to me that a substitute lesson with a different teacher would also take place in a new room. I don't feel like assuming otherwise is a safe assumption to make.
Please also change the German String resource value.
Can you also omit all dots (.) in the lessons, as that would look more clean?
I see you made a change to the filterUserFilters() method. Unfortunately, if I'm not mistaken, it is wrong: just because no class information is available doesn't mean that it matches the class filter! Instead, if enabled by the user in the preferences, it will already pass when it is an info-only entry. (Your plan doesn't seem to have any info-only entries, though.)
Also, I fail to observe after all how it was required to make the classInClassadjective case-insensitive as your plan seems consistent about capitalization to me (though of course I only have access to one example file), but I now realized that it doesn't affect the classInInfo adjective that I was primarily concerned about.
[`6be17a189f`][6be17a189f] is not really what I suggested, but I suppose it works as well… just `emailTheDev(getActivity());` would have sufficed, though, instead of the whole if clause.
To explain why it doesn't matter: `emailTheDev()` is asking for a `Context`. `Activity` extends `Context`. So `getActivity()` will be implicitly cast to a `Context` because that's what is asked for. We could also cast it explicitly with `(Context) getActivity()`, but it would require more typing.
---
I didn't realize the omission was intentional… can you be sure that only the teacher changed and that it is not also in another room? It would make sense to me that a substitute lesson with a different teacher would also take place in a new room. I don't feel like assuming otherwise is a safe assumption to make.
---
> * Please also change the German String resource value.
> * Can you also omit all dots (`.`) in the lessons, as that would look more clean?
---
I see you made [a change][filter] to the `filterUserFilters()` method. Unfortunately, if I'm not mistaken, it is wrong: just because `no class information is available` doesn't mean that it matches the class filter! Instead, if enabled by the user in the preferences, it will already [`pass` when it is an info-only entry][info-only]. (Your plan doesn't seem to have any info-only entries, though.)
Also, I fail to observe after all how it was required to make the `classInClass` `adjective` case-insensitive as your plan seems consistent about capitalization to me (though of course I only have access to one example file), but I now realized that it doesn't affect the `classInInfo` adjective that I was primarily concerned about.
[info-only]: https://notabug.org/JasMich.de/DSBDirect/src/6be17a189fade2a31fc98a9aab8b81111ae56267/app/src/main/java/godau/fynn/dsbdirect/table/reader/Reader.java#L134
[filter]: https://notabug.org/JasMich.de/DSBDirect/src/6be17a189fade2a31fc98a9aab8b81111ae56267/app/src/main/java/godau/fynn/dsbdirect/table/reader/Reader.java#L126
[6be17a189f]: https://notabug.org/JasMich.de/DSBDirect/commit/6be17a189fade2a31fc98a9aab8b81111ae56267
[bc9b2d07c2]: https://notabug.org/JasMich.de/DSBDirect/commit/bc9b2d07c2657cdad6ef0f8ed7fdde7896455061
I have added the locations that build outputs are located to .gitignore. Your pull request wants to add one blank line at the end of .gitignore, that's what causes the merge conflict. But don't worry about it, I will squash your commits together into one anyway and can easily resolve the conflict in the process.
I have added the locations that build outputs are located to `.gitignore`. Your pull request wants to add one blank line at the end of `.gitignore`, that's what causes the merge conflict. But don't worry about it, I will squash your commits together into one anyway and can easily resolve the conflict in the process.
Okay, I updated the strings for Goethe and fixed the fix of emailTheDev(getActivity()).
I added the Room to the info column if necessary.
To the case insensitivity: That's right, in the example file, the class letters are upper-case. But sometimes, they are added in lowercase. It changes every few weeks/days depending on the person creating the substitution table. As I do not want to patch this every two weeks I would like to take the save way by using case insensitivity.
Still any other things to fix or is it ready to pull?
Okay, I updated the strings for `Goethe` and fixed the fix of `emailTheDev(getActivity())`.
I added the Room to the info column if necessary.
To the case insensitivity: That's right, in the example file, the class letters are upper-case. But sometimes, they are added in lowercase. It changes every few weeks/days depending on the person creating the substitution table. As I do not want to patch this every two weeks I would like to take the save way by using case insensitivity.
*Still any other things to fix or is it ready to pull?*
I merged it manually with 81500dd0ed, attributing the squash commit to you for statistics and such.
I merged it manually with [`81500dd0ed`][81500dd0ed], attributing the squash commit to you for statistics and such.
[81500dd0ed]: https://notabug.org/fynngodau/DSBDirect/commit/81500dd0ed05c7cd0c0d591f900b6e762142db1a
I quickly need to find a place to credit you first, as required by the GNU GPL v3 that you licensed the GoetheHamburg.java file under. Then we can release, yes.
I quickly need to find a place to credit you first, as required by the GNU GPL v3 that you licensed the `GoetheHamburg.java` file under. Then we can release, yes.
After creating a tag starting with release-, we need to be patient for a few days and wait for the fdroid build server to include DSBDirect in its next round of builds that gets pushed to the repo once all apps are done. Nothing can be done by us to speed up the process.
After creating a tag starting with `release-`, we need to be patient for a few days and wait for the fdroid build server to include DSBDirect in its next round of builds that gets pushed to the repo once all apps are done. Nothing can be done by us to speed up the process.
I wrote a parser for Goethe-Gymnasium Hamburg (
189802). For that, I had to change the following in general files:Utility.javaReader.java(required by Goethe's timetables)Thank you a lot for your effort!
Having
classInClasscase-sensitive improved accuracy by a little when filtering info-only entries (especially for schools where the class letter is capital). Could you explain a little more why this change is necessary?Looks like merging this would mess with the changes of #18. Rebasing your
essentialsbranch onto thegoethebranch could probably fix this (I'm not really sure though).In Goethe's substitute tables, the classes letters are sometimes upper case and sometimes lowercase. Same for teacher's names.
The merge conflict seems to be fixed now...
Btw. How is your release management? If you approve this pull, is it released automatically or does it take some time?
In case displaying an entry fails, you are currently initializing an entry with empty Strings. I'd prefer it if you could pass
nullinstead as allEntry()parameters are annotated as@Nullable.Furthermore, there exists a String resource,
error_parsing_partial, and it is used asinfovalue when a whole page fails (illogically, then, even if it is the only page…). Maybe the wording can be changed from "Some data could not be parsed" to account for your case where only a single entry fails, but I'm not sure to what.Release is whenever I say so. This can gladly be after this pull request is merged. F-Droid propagation would take some time as usual, and Play releases are also not pushed immediately.
It is quite the manual process, so I usually wait until a few fixes have accumulated.
Okay, I'm now passing
null.I don't understand why to change
error_parsing_partial. That's actually what happened in that case: Only some parts failed to load.Concerning case sensitivity: since the gain in accuracy is minimal anyway, it's probably not worth it to differentiate based on what parser has parsed the plan here. A more effective filter for info entries is needed anyway. Tracking that effort in #21.
Thanks for little change again. I was thinking, in case a lot of entries fail to parse in a row, it would make more sense to say something like "We're missing an entry here" many times instead of "some data", but nevermind. If it's broken, it's going to look broken either way.
Oh, I didn't notice you are now passing
row.text(). That also works, of course.If you open the preferences screen, you'll notice that the Parser preference has been blocked because There is no choice. I'll simply revert the commit that added this (
c81b3a3023) after merging this pull request.Could you please send me html files from Goethe for testing?
In order for the preference in the preference screen to allow selecting Goethe manually, you need to add it to the two String array resources here. The Röntgen option should be removed, but I forgot to do that. If I do it now, it will only cause a merge conflict, so I'm asking you to simply replace it with Goethe.
I see you're trying to fit the subject into the lesson field (
(\d.* Std\. \w+)). However, the lesson field is intended to be very small, usually only one number, though it can of course also hold a duration (1-10).1.-5.Std.Musikis unfortunately way too long and breaks the cards layout as seen in the attachment. I thinkStd.is redundant – it already is in the lesson column – and every.can be omitted to improve readability. The subject should go somewhere else.In the Untis parser, I am concatenating class and subject using a
·character. This may not be a wise choice for you as well, as space for class is very limited in list view (the Untis plan I've seen had lesson abbreviated). I think it would make more sense for you to move the subject to the info field. I imagine the parser to make this8B | 1. - 2. Std. Sport bei MAN im Raum ESH fällt ausbecome this (
affectedClass | lesson | replacementTeacher | info):8B | 1-2 | MAN | Sport im Raum ESH fällt ausAnother thing I've noticed is that your plan is not actually school-specific, but rather generally a
HeinekingMedia Vertretungsplanas indicated in thetitle. I've seen one before searching around on the internet (archive), and it looked very similar. If you can add support for multiple days in one page, we can dynamically detect anyHeinekingMedia Vertretungsplanand use the parser for that automatically. But we can finish this pull request first and add more general support and detection at a later date if you prefer.However, this should be reflected in the UI (the parser selection should show
HeinekingMediainstead ofGoethe).Okay, I fixed the langth of lessons and moved the subject into the information column. I think I updated the String from
RöntgentoGoethebut it anyway allways shows meThere is no choice, only Untis boards can be parsed at the moment.I agree, the general support for
HeinekingMedia Vertretungsplanwe could improve later.Btw. I fixed a little bug concerning Android 4 devices in your mailing function.
c81b3a3023that causes blocking the preferences after merging.Can you also omit all dots (
.) in the lessons, as that would look more clean?Comparing Strings with
!=will not work. You need to use.equals:Furthermore, I noticed you are omitting information that starts with the keyword
im Raum:Original:
9A | 6. Std. Religion bei FER im Raum H002 statt bei LIEExpected result:
9A | 6 | FER | Religion im Raum H002 statt bei LIEActual result:
9A | 6. | FER | Religion statt bei LIESince it's quite the critical information, please fix this as well.
Fixed comparing of the strings.
That's wanted. If the text contains
statt beiand a teacher's short code (XXX), it means the room didn't change but only the teacher. (Look at the teacher column: it's unequal to the short code in the info text.) That's why I'm removing the room in that case.6be17a189fis not really what I suggested, but I suppose it works as well… justemailTheDev(getActivity());would have sufficed, though, instead of the whole if clause.To explain why it doesn't matter:
emailTheDev()is asking for aContext.ActivityextendsContext. SogetActivity()will be implicitly cast to aContextbecause that's what is asked for. We could also cast it explicitly with(Context) getActivity(), but it would require more typing.I didn't realize the omission was intentional… can you be sure that only the teacher changed and that it is not also in another room? It would make sense to me that a substitute lesson with a different teacher would also take place in a new room. I don't feel like assuming otherwise is a safe assumption to make.
I see you made a change to the
filterUserFilters()method. Unfortunately, if I'm not mistaken, it is wrong: just becauseno class information is availabledoesn't mean that it matches the class filter! Instead, if enabled by the user in the preferences, it will alreadypasswhen it is an info-only entry. (Your plan doesn't seem to have any info-only entries, though.)Also, I fail to observe after all how it was required to make the
classInClassadjectivecase-insensitive as your plan seems consistent about capitalization to me (though of course I only have access to one example file), but I now realized that it doesn't affect theclassInInfoadjective that I was primarily concerned about.I have added the locations that build outputs are located to
.gitignore. Your pull request wants to add one blank line at the end of.gitignore, that's what causes the merge conflict. But don't worry about it, I will squash your commits together into one anyway and can easily resolve the conflict in the process.Okay, I updated the strings for
Goetheand fixed the fix ofemailTheDev(getActivity()).I added the Room to the info column if necessary.
To the case insensitivity: That's right, in the example file, the class letters are upper-case. But sometimes, they are added in lowercase. It changes every few weeks/days depending on the person creating the substitution table. As I do not want to patch this every two weeks I would like to take the save way by using case insensitivity.
Still any other things to fix or is it ready to pull?
I think it finally looks good! Thank you!
I merged it manually with
81500dd0ed, attributing the squash commit to you for statistics and such.Could you create a release?
I quickly need to find a place to credit you first, as required by the GNU GPL v3 that you licensed the
GoetheHamburg.javafile under. Then we can release, yes.I have credited you below where I put my own name, I hope it's alright.
Release 2.2 is tagged!
All right. Thank you.
I saw the update in Google Play. What's about F-Droid? It keeps showing version
2.1.After creating a tag starting with
release-, we need to be patient for a few days and wait for the fdroid build server to include DSBDirect in its next round of builds that gets pushed to the repo once all apps are done. Nothing can be done by us to speed up the process.