| [11:03:17] | * farmBOT has joined #farmos |
| [11:55:42] | <mstenta[m]> | farmOS dev call in 5 minutes! |
| [11:55:54] | <mstenta[m]> | https://meet.jit.si/farmos-dev |
| [12:00:35] | <paul121[m]> | Oops lost track of time be there soon |
| [14:58:10] | <mstenta[m]> | paul121 symbioquine: at the risk of beating this dead horse any more... now i'm wondering if we should change the quantity `csv` view mode to `plain` |
| [14:58:18] | <mstenta[m]> | and maybe put it in the quantity module itself? |
| [14:59:37] | <mstenta[m]> | then we wouldn't need the extra farm_csv module (yet), and it feels like the view mode would better describe what it's actually doing (and be more generalized) |
| [15:07:25] | <mstenta[m]> | or plain_text like the Drupal core text formatter |
| [15:12:05] | <symbioquine[m]> | I like the idea of putting the whitespace removal code in the quantity module if that doesn't create some sort of circular dependency... |
| [15:13:35] | <mstenta[m]> | i think it would be pretty easy to... might give it a quick try and push a single commit to show |
| [15:13:47] | <mstenta[m]> | (not on the same branch, so it's not cluttered in the PR) |
| [15:14:10] | <mstenta[m]> | i can't help but still feel unsatisfied with the PR |
| [15:14:21] | <symbioquine[m]> | The name change makes sense too, because the naming still works for a tsv file or similar then too. |
| [15:14:35] | <symbioquine[m]> | s/too/also/ |
| [15:15:37] | <mstenta[m]> | true |
| [15:16:09] | <mstenta[m]> | yea, i feel like csv is too misleading |
| [15:16:10] | <mstenta[m]> | it's really just creating a plain text view of the quantity |
| [15:16:23] | <mstenta[m]> | (a one-line plain-text view to be exact... but i think plain_text is sufficient) |
| [15:26:35] | <mstenta[m]> | yea actually... I think I like this better now that I see it |
| [15:29:18] | <mstenta[m]> | https://github.com/mstenta/farmOS/commit/a1e21c54bd3ba53c7d85f9c39008b99... |
| [15:30:36] | <mstenta[m]> | what this does is, rather than using a hook to add a #post_render callback to quantities, it extends the EntityViewBuilder class in the quantity module itself, so it is a core feature of Quantity entities essentially |
| [15:30:49] | <mstenta[m]> | a "plain text" view mode |
| [15:31:58] | <paul121[m]> | that's grea! |
| [15:31:58] | <mstenta[m]> | (full disclosure: i just sketched up the commit... actually testing to see if it works now... 😄) |
| [15:31:59] | <paul121[m]> | * that's great! |
| [15:45:34] | <mstenta[m]> | It works! |
| [15:46:39] | <mstenta[m]> | Oh this feels much better. I think we've resolved the "cruft" issue! |
| [15:46:42] | <mstenta[m]> | Even though it's essentially the same thing... just naming it properly and putting it in the right place makes all the difference. |
| [15:56:47] | <mstenta[m]> | OK, I made a fresh branch cleaned up to remove all the other commits: https://github.com/farmOS/farmOS/compare/2.x...mstenta:farmOS:2.x-csv-plain |
| [15:57:08] | <mstenta[m]> | OK if I force push this to replace the branch in the current PR and then merge? |
| [17:13:01] | <paul121[m]> | <mstenta[m]> "OK if I force push this to..." <- Yes, although I think we need an update hood to create the new view mode |
| [17:13:18] | <paul121[m]> | * Yes, although I think we need an update hook to create the new view mode |
| [17:16:12] | <mstenta[m]> | Oh yes! Good thing I had that logic in a previous commit (I think) :-) |
| [17:16:18] | <mstenta[m]> | Thanks I almost forgot that |
| [17:16:38] | <mstenta[m]> | I'll update that and force push when I have a chance |