Re: [hatari-devel] Hatari screen dialog regression

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Sat, 16 Aug 2025 07:01:35 +0200
schrieb Andreas Grabher <andreas_g86@xxxxxxxxxx>:

> > Am 13.08.2025 um 22:15 schrieb Andreas Grabher <andreas_g86@xxxxxxxxxx>:
> >   
> >> Am 13.08.2025 um 21:37 schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> >> 
> >> Hi,
> >> 
> >> On 13.8.2025 22.05, Andreas Grabher wrote:  
> >>>> Am 11.08.2025 um 01:11 schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> >>>> On 10.8.2025 20.53, Andreas Grabher wrote:  
> >>>>>> Von: Andreas Grabher <andreas_g86@xxxxxxxxxx>  
> >>>>>>>> Am 07.08.2025 um 22:21 schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:  
> >>>>>>> I just checked the patches to see what would need to be merged into Previous. I recognised that configuration handling is done in paths.c now. I don’t think it is a good idea to increase dependencies this way. It would be better to create a function in configuration like Configuration_GetScreenShotDir() which returns either the default or the user selected directory. That way paths.c can be untouched and code structure is more logical.
> >>>>>>> 
> >>>>>>> Hatari has a quite good code structure, but it seems to get eroded more and more over time. I think it is a win-win situation if portability is taken into account when editing the source files. (H)Atari-specific code should not go into common files.  
> >>>>> 
> >>>>> Could you propose a patch?  
> >>> I appended the requested patch. You know that your code structure has improved if the amount of includes is reduced.  
> >> 
> >> I don't think this is improvement.  It changes Paths_GetScreenShotDir() function into one that should not be called, without documenting that, or changing the function name to match its new functionality.
> >> 
> >> At least its name should be changed to Paths_GetScreenShotDefaultDir(), and comment be updated to state that Configuration_GetScreenShotDir() should be called instead.
> >> 
> >> Please provide patch with "git format-patch" so that you get correct attribution for the change!
> >> 
> >> 
> >>    - Eero
> >>   
> > Appended is an improved patch with new function name. As mentioned I have no git here. But the patch can be easily applied using
> > 
> > patch -p1 -i /path/to/paths_fix2.diff
> 
> Does the fact that 2.6.1 has been released mean, that my patch (which I made on your request) is ignored?

 Hi!

The patch looks fine to me! Eero, do you have any objections? Otherwise,
I'll commit it.

 Thomas