search

sephii / taxi

Taxi is a CLI to time tracking backends
https://taxi-timesheets.readthedocs.io/
Do What The F*ck You Want To Public License
41 stars 15 forks source link

Allow activity.id + project.id to be strings, allow aliases to be written in uppercase in timesheet entries #133

Closed gnutix closed 4 years ago

gnutix commented 4 years ago

Fix #129

The tests are passing, but I don't know how much they really cover. I've tested locally every command listed when running taxi with as many options as possible, but only with my current setup (for an alias infra-42 = INFRA/42, project.id = "INFRA" and activity.id = 42).

@sephii it would be nice if you could try it out with Zebra's setup to make sure it has not broken anything.

sephii commented 4 years ago

Thanks for the PR! I made some changes to centralize the lowercase mechanism to the aliases database. Can you please check this branch and let me know if it works for you? https://github.com/liip/taxi/tree/issue-129-patch

sephii commented 4 years ago

By the way, this causes errors with taxi-zebra, because it expects projects and activities ids to be integers.

sephii commented 4 years ago

I didn't really like the idea of having the aliases being case insensitive, so I gave all this more thought.

  1. I don't know why the "shared aliases" are stored in the configuration file. This might be historical. There's no reason to put them there since they're already in the projects database (that's in json format).
  2. Since json is case sensitive, removing the shared aliases from the configuration file and using the ones in the projects database instead should fix the issue. Along with this, replacing the config file format with toml, which is case sensitive, would allow to override the shared aliases (while keeping the case).

1 is already done on the issue-129-take2 branch. 2 shouldn't be that difficult to do.

sephii commented 4 years ago

Closing this since it was resolved by https://github.com/liip/taxi/commit/c56a3bec8516906fc151c0f8d8d826cf5d1c08ad. I've opened #137 to make it possible to override non-lowercase aliases.